-
Notifications
You must be signed in to change notification settings - Fork 170
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
quiet logger #482
quiet logger #482
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 6a83e06 in 52 seconds
More details
- Looked at
100
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. r2r/core/logging/kv_logger.py:64
- Draft comment:
The removal of initialization logging in individual logging providers and centralizing it inKVLoggingSingleton.configure
is a good approach to reduce log redundancy and centralize configuration logging. Ensure that this change aligns with the overall logging strategy of the application. - Reason this comment was not posted:
Confidence changes required:0%
The PR removes the initialization logging from the LocalKVLoggingProvider and PostgresKVLoggingProvider constructors. This might reduce the verbosity of logs, which could be the intent given the PR title 'quiet logger'. However, it introduces a new logging statement in the KVLoggingSingleton.configure method, which logs the initialization with the configuration. This change centralizes the logging of initialization to one place, which could be beneficial for maintaining a single point of logging the configuration. The change seems to be consistent across different parts of the code, aiming to reduce redundancy and centralize logging.
2. r2r/core/pipeline/base_pipeline.py:33
- Draft comment:
The introduction of thelog_run_info
parameter to control logging at the pipeline level is a thoughtful addition. It allows for better control over logging verbosity, especially in complex setups with nested pipelines. Ensure that all uses of this parameter are intentional and correctly implemented across different pipeline configurations. - Reason this comment was not posted:
Confidence changes required:0%
The PR introduces a new parameterlog_run_info
in thePipeline
constructor to control whether run information should be logged. This is a useful feature for controlling verbosity on a per-pipeline basis. The default value is set toTrue
, which maintains the existing behavior unless explicitly overridden. This change is applied consistently in therun
method of thePipeline
class, where it checks thelog_run_info
flag before logging the run information. Additionally, when creating sub-pipelines for knowledge graph (KG) and embedding within theIngestionPipeline
, thelog_run_info
is explicitly set toFalse
. This could be intended to prevent redundant logging when these pipelines are part of a larger pipeline, which already logs the run information.
Workflow ID: wflow_IkG0EWPqI2qwIuvW
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
* quiet logger (#482) * Add debug mode to Posthog (#480) * Feature/add kg agent search to pipeline rebased 2 (#483) * expand search pipeline * checkin progress * checkin progress * grinding out * make sure we can configure kg * fix id error on server use * checkin * finish merge, add rag stream * cleanup and pass tests * up * harmonize * check in config refactor * checkin work * cleanup * minor cleanups * Add optional metadata to KG cookbook (#484) * Add optional metadata to KG cookbook * Align CB with script * rm cruft and fix bug (#488) * Feature/refactor app logic (#490) * modularize app logic * add management service * add management service * add management service * Feature/move to singleton to prevent reload rebased (#491) * modularize app logic * add management service * prevent r2r reload * prevent r2r reload * add back file (#492) * add back file (#493) * add back file * fix bugs * fix the routes (#496) * rebase * update docs * cleanups --------- Co-authored-by: Nolan Tremelling <34580718+NolanTrem@users.noreply.github.com>
* quiet logger (SciPhi-AI#482) * Add debug mode to Posthog (SciPhi-AI#480) * Feature/add kg agent search to pipeline rebased 2 (SciPhi-AI#483) * expand search pipeline * checkin progress * checkin progress * grinding out * make sure we can configure kg * fix id error on server use * checkin * finish merge, add rag stream * cleanup and pass tests * up * harmonize * check in config refactor * checkin work * cleanup * minor cleanups * Add optional metadata to KG cookbook (SciPhi-AI#484) * Add optional metadata to KG cookbook * Align CB with script * rm cruft and fix bug (SciPhi-AI#488) * Feature/refactor app logic (SciPhi-AI#490) * modularize app logic * add management service * add management service * add management service * Feature/move to singleton to prevent reload rebased (SciPhi-AI#491) * modularize app logic * add management service * prevent r2r reload * prevent r2r reload * add back file (SciPhi-AI#492) * add back file (SciPhi-AI#493) * add back file * fix bugs * fix the routes (SciPhi-AI#496) * rebase * update docs * cleanups --------- Co-authored-by: Nolan Tremelling <34580718+NolanTrem@users.noreply.github.com>
Summary:
Reduced logging verbosity in
KVLoggingProvider
classes and added alog_run_info
flag to thePipeline
class for conditional logging.Key points:
LocalKVLoggingProvider
andPostgresKVLoggingProvider
constructors inr2r/core/logging/kv_logger.py
.KVLoggingSingleton.configure
method inr2r/core/logging/kv_logger.py
.KVLoggingSingleton.log
method to include log details inr2r/core/logging/kv_logger.py
.log_run_info
flag inPipeline
class constructor inr2r/core/pipeline/base_pipeline.py
.log_run_info
flag inPipeline.run
method inr2r/core/pipeline/base_pipeline.py
.log_run_info=False
for nested pipelines inIngestionPipeline.add_pipe
method inr2r/core/pipeline/base_pipeline.py
.Generated with ❤️ by ellipsis.dev