Skip to content
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

Remove 'R2R_PR2R_PROJECT_NAME' log #2000

Merged
merged 1 commit into from
Feb 22, 2025
Merged

Conversation

NolanTrem
Copy link
Collaborator

@NolanTrem NolanTrem commented Feb 22, 2025

Not sure why/how this was here.


Important

Remove redundant logging of R2R_PR2R_PROJECT_NAME and adjust line breaks for consistency in app_entry.py and logging_config.py.

  • Logging:
    • Remove redundant logging of R2R_PR2R_PROJECT_NAME in app_entry.py.
  • Formatting:
    • Adjust line breaks for logging statements in app_entry.py and logging_config.py for consistency and readability.

This description was created by Ellipsis for 5b37b75. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested. Reviewed everything up to 5b37b75 in 1 minute and 45 seconds

More details
  • Looked at 71 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 4 drafted comments based on config settings.
1. py/core/main/app_entry.py:82
  • Draft comment:
    Corrected the logging order: removed duplicate logging of a wrongly named env var and now logs R2R_POSTGRES_PORT and R2R_POSTGRES_PASSWORD correctly.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is purely informative, describing what was changed in the code without providing any actionable feedback or suggestions. It does not align with the rules for useful comments, as it does not ask for confirmation of intent, suggest improvements, or identify potential issues.
2. py/core/utils/logging_config.py:74
  • Draft comment:
    Formatting improvements: multiline formatting for log_console_formatter and JSON formatter settings enhance readability.
  • Reason this comment was not posted:
    Confidence changes required: 20% <= threshold 50%
    None
3. py/core/utils/logging_config.py:74
  • Draft comment:
    Multiline formatting for log_console_formatter and the JSON formatter improves readability.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
4. py/core/utils/logging_config.py:10
  • Draft comment:
    There's an incomplete sentence in the HTTPStatusFilter docstring. The sentence "searches for HTTP status codes and adjusts the." seems to be missing what is being adjusted (likely the record's log level). Please update the docstring to clarify this.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_dVlqILFx1Geic0O7


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

)
logging.info(
f"Environment R2R_PROJECT_NAME: {os.getenv('R2R_PR2R_PROJECT_NAME')}"
f"Environment R2R_POSTGRES_PASSWORD: {os.getenv('R2R_POSTGRES_PASSWORD')}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the duplicate/wrong env var log (R2R_PR2R_PROJECT_NAME). Logging the DB password may expose sensitive info in production. Consider redacting or removing sensitive credentials.

Suggested change
f"Environment R2R_POSTGRES_PASSWORD: {os.getenv('R2R_POSTGRES_PASSWORD')}"

@NolanTrem NolanTrem merged commit 7ab5f3e into main Feb 22, 2025
16 checks passed
@NolanTrem NolanTrem deleted the Nolan/R2R_PR2R_PROJECT_NAME branch February 22, 2025 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant