Refactor: Migrate from print statements to Python's logging module#4
Refactor: Migrate from print statements to Python's logging module#4
Conversation
This commit replaces all instances of `print()` with calls to Python's standard `logging` module. Key changes: - Configured a logger in `sbg.py` with a standard format and INFO level. - Replaced informational, warning, and error `print()` calls with `logger.info()`, `logger.warning()`, and `logger.error()` respectively. - Handled `sys.exit()` calls to log critical errors or informational messages (like user abortion) before exiting. - Updated unit tests in `test_sbg.py` to use `self.assertLogs` for capturing and verifying log output, ensuring continued test coverage. - Added a note to `README.md` about the use of the logging module. This change provides more structured and configurable logging, allowing you to control log verbosity and output destinations (e.g., file output) if needed in the future.
|
Warning Rate limit exceeded@tutunak has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 25 minutes and 51 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe changes introduce standardized logging throughout the codebase by replacing all direct print statements with Python's logging module. The README is updated to document this logging approach. Test cases are refactored to capture and assert log messages using unittest's assertLogs, replacing manual stdout redirection. Changes
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
This commit updates logger calls in `sbg.py` to use %-formatting
with positional arguments (e.g., `logger.info("Message: %s", arg)`)
instead of f-strings.
This change adheres to logging best practices, as it avoids unnecessary
string formatting if the log message's level is not high enough to be
emitted.
Unit tests in `test_sbg.py` have been confirmed to pass with these
changes, as the final log output remains the same.
Docstrings generation was requested by @tutunak. * #4 (comment) The following files were modified: * `sbg.py`
|
Note Generated docstrings for this pull request at #5 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
sbg.py (1)
98-98: Consider the impact offorce=Trueon library usageThe
force=Trueparameter will override any existing logging configuration, which could be problematic if this script is ever imported as a module by other applications.Consider making the logging configuration conditional or allow it to be disabled:
- logging.basicConfig(level=logging.INFO, format='%(asctime)s - %(levelname)s - %(message)s', force=True) + if not logging.getLogger().handlers: + logging.basicConfig(level=logging.INFO, format='%(asctime)s - %(levelname)s - %(message)s')Alternatively, you could add a parameter to control logging configuration for better modularity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md(1 hunks)sbg.py(4 hunks)test_sbg.py(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
test_sbg.py (1)
sbg.py (2)
gather_all_projects(61-79)clone_or_pull(82-95)
🔇 Additional comments (11)
sbg.py (6)
6-6: LGTM: Proper logging setupThe logging import and logger initialization follow Python best practices by using
__name__for the logger name, which creates a hierarchical logger namespace.Also applies to: 9-9
74-74: LGTM: Appropriate warning level for HTTP errorsUsing
logger.warning()for HTTP errors during group fetching is appropriate, as these are non-fatal issues that the user should be aware of but don't stop the overall process.
85-85: LGTM: Appropriate info level for operational messagesUsing
logger.info()for clone/pull operations provides good visibility into the script's progress without being overly verbose.Also applies to: 91-91
89-89: LGTM: Appropriate error level for subprocess failuresUsing
logger.error()for git command failures is correct, as these are actual errors that prevent successful operation on specific repositories.Also applies to: 95-95
105-105: LGTM: Good progress reportingThe informational logging provides clear feedback about the script's progress, including group processing and total project counts.
Also applies to: 108-108
133-137: LGTM: Proper exception handling with appropriate loggingThe exception handling correctly uses:
logger.info()for user interruption (not an error)logger.critical()for fatal exceptions- Appropriate exit codes (0 for user abort, 1 for fatal errors)
This maintains the same behavior as before while providing structured logging.
test_sbg.py (4)
9-9: LGTM: Proper import for logging in testsAdding the logging import is necessary for the test updates to capture and assert on log messages.
43-44: LGTM: Proper logging configuration for testsSetting the logger level to INFO ensures that log messages will be captured by
assertLogs. The comment clearly explains the purpose of this configuration.
137-142: LGTM: Proper migration to assertLogs for warning captureThe test correctly:
- Uses
assertLogswith the specific logger and WARNING level- Maintains the same assertion logic by checking for the expected warning message
- Verifies both the functional behavior (2 projects returned) and the logging behavior
This properly tests the new logging implementation while maintaining test coverage.
172-175: LGTM: Proper migration to assertLogs for error captureThe test correctly captures ERROR level logs and verifies the expected error message content. The assertion properly checks that the clone failure message is logged.
README.md (1)
112-116: LGTM: Clear and helpful logging documentationThe new "Logging" section effectively:
- Explains the adoption of Python's logging module
- Highlights the benefits of structured logging (verbosity control, output destinations)
- Clearly states the default behavior (INFO level to stdout)
- Provides users with useful information about the logging approach
This documentation aligns well with the implementation changes and helps users understand the new logging behavior.
Docstrings generation was requested by @tutunak. * #4 (comment) The following files were modified: * `sbg.py`
1 participant
This commit replaces all instances of
print()with calls to Python's standardloggingmodule.Key changes:
sbg.pywith a standard format and INFO level.print()calls withlogger.info(),logger.warning(), andlogger.error()respectively.sys.exit()calls to log critical errors or informational messages (like user abortion) before exiting.test_sbg.pyto useself.assertLogsfor capturing and verifying log output, ensuring continued test coverage.README.mdabout the use of the logging module.This change provides more structured and configurable logging, allowing you to control log verbosity and output destinations (e.g., file output) if needed in the future.
Summary by CodeRabbit
Documentation
Refactor
Tests