-
Notifications
You must be signed in to change notification settings - Fork 252
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add a global project config #1333
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Reviewed everything up to 581f231 in 30 seconds
More details
- Looked at
199
lines of code in8
files - Skipped
0
files when reviewing. - Skipped posting
6
drafted comments based on config settings.
1. py/core/base/logging/run_logger.py:347
- Draft comment:
Consider retaining the fallback toos.getenv("R2R_PROJECT_NAME", "default")
for backward compatibility in caseconfig.app.project_name
is not set. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The code change already includes a fallback mechanism usingos.getenv("R2R_PROJECT_NAME", "default")
ifconfig.app.project_name
is not set. The comment seems to suggest something that is already implemented in the code. Therefore, the comment is redundant and does not require any further action.
I might be missing some context about why the comment was made, but based on the code change, it seems the fallback is already implemented. The comment might have been made before the code was updated to include the fallback.
The code clearly shows the fallback mechanism is in place, so the comment is unnecessary. The comment might have been made based on an outdated version of the code.
The comment is redundant because the code already includes the suggested fallback mechanism. Therefore, the comment should be deleted.
2. py/core/base/logging/run_logger.py:586
- Draft comment:
Theconfigure
method no longer has a defaultLoggingConfig
. Ensure this change is documented as it requires explicit configuration now. - Reason this comment was not posted:
Confidence changes required:50%
Theconfigure
method inRunLoggingSingleton
was changed to remove the defaultLoggingConfig
argument. This is a good change as it enforces explicit configuration, but it should be documented in the method docstring or comments.
3. py/core/base/providers/base.py:21
- Draft comment:
The addition ofapp: AppConfig
toProviderConfig
is consistent with the new global project config approach. Ensure all providers are updated to handle this new field. - Reason this comment was not posted:
Confidence changes required:30%
The addition ofAppConfig
to theProviderConfig
class inbase.py
is consistent with the new global project config approach. This change is correctly reflected in theR2RConfig
class inconfig.py
.
4. py/core/base/providers/base.py:9
- Draft comment:
Ensure thatAppConfig
includes all necessary fields for global app settings and that these fields are well-documented. - Reason this comment was not posted:
Confidence changes required:40%
TheAppConfig
class is introduced to handle global app settings, which is a good addition. However, ensure that all necessary fields are included and documented.
5. py/core/main/config.py:124
- Draft comment:
The integration ofAppConfig
intoR2RConfig
is well-implemented. Ensure that all sections that requireapp
are updated accordingly. - Reason this comment was not posted:
Confidence changes required:30%
TheAppConfig
is correctly integrated into theR2RConfig
class, and thecreate
method is used to instantiate it. This ensures consistency across the configuration setup.
6. py/core/providers/database/postgres.py:75
- Draft comment:
Consider retaining the fallback toos.getenv("R2R_PROJECT_NAME")
for backward compatibility in caseconfig.app.project_name
is not set. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_9QU0EipGcsJgER7e
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
* fix issues in compose (#1324) * Update Hatchet compose setup (#1327) * add ingest chunks workflows (#1329) * add ingest chunks workflows * finalize ingest chunks * fix * Allow for exclusion of postgres (#1328) * update type checks (#1331) * Limiting KG Creation Scheduling (#1332) * try * up * up * space * add it in ingestion * rm ingestion * up * bump pkg * add a global project config (#1333) * Corrections for KG SDK usage (#1330) * Corrections for KG SDK usage * Clean up * missed file * Adding a completions endpoint (#1326) * up * rm unnecessary import * add completion model * up * Update JS (#1335) --------- Co-authored-by: Nolan Tremelling <[email protected]> Co-authored-by: Shreyas Pimpalgaonkar <[email protected]>
Important
Adds a global project configuration using
AppConfig
for centralized project settings across logging, database, and configuration modules.AppConfig
class inbase.py
for global project settings.R2RConfig
inconfig.py
to includeapp: AppConfig
.[app]
section inr2r.toml
for project-wide settings.PostgresRunLoggingProvider
inrun_logger.py
to useconfig.app.project_name
.configure()
method inRunLoggingSingleton
to requireLoggingConfig
.PostgresDBProvider
inpostgres.py
to useconfig.app.project_name
.app: AppConfig
field toProviderConfig
inbase.py
.AppConfig
in__init__.py
for providers.This description was created by for 581f231. It will automatically update as commits are pushed.