Skip to content

do not log to file if progress#5402

Merged
adhami3310 merged 1 commit intomainfrom
do-not-log-to-file-if-progress
Jun 2, 2025
Merged

do not log to file if progress#5402
adhami3310 merged 1 commit intomainfrom
do-not-log-to-file-if-progress

Conversation

@adhami3310
Copy link
Copy Markdown
Member

No description provided.

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Modifies debug logging behavior to prevent duplicate file logging during progress tracking operations, but introduces a potential issue with parameter handling.

  • Adds condition kwargs.pop('progress', None) is None in reflex/utils/console.py to prevent file logging when progress tracking is active
  • Potential bug: kwargs.pop() removes the 'progress' key which could break code that needs this parameter later
  • Consider using kwargs.get() instead of pop() to preserve the parameter for subsequent operations

💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!

1 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile

Comment thread reflex/utils/console.py
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Jun 2, 2025

CodSpeed Performance Report

Merging #5402 will not alter performance

Comparing do-not-log-to-file-if-progress (f4175e6) with main (95819ab)

Summary

✅ 8 untouched benchmarks

@adhami3310 adhami3310 merged commit 8fb0610 into main Jun 2, 2025
42 checks passed
@adhami3310 adhami3310 deleted the do-not-log-to-file-if-progress branch June 2, 2025 18:26
adhami3310 added a commit that referenced this pull request Jun 2, 2025
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.

2 participants