Skip to content

Issue 68 logging#73

Open
kan-fu wants to merge 4 commits intomainfrom
issue-68-logging
Open

Issue 68 logging#73
kan-fu wants to merge 4 commits intomainfrom
issue-68-logging

Conversation

@kan-fu
Copy link
Copy Markdown
Collaborator

@kan-fu kan-fu commented Apr 23, 2026

a folllow-up pr based on #69

The most significant change in this series of commits is the replacement of direct print() statements and warnings.warn() calls with a structured, hierarchical logging system.

Mapping Logic

The migration follows a consistent mapping from the previous console output to the new logging levels:

Previous System New Logging Level Description
print() (Progress/Status) INFO Standard user-facing messages (e.g., download progress).
print() (Debug/Metadata) DEBUG Detailed technical info controlled by showInfo (e.g., requested URLs, response times).
warnings.warn() WARNING API-returned warnings and non-fatal issues.
Console Error Output ERROR API errors and HTTP failure details.

Custom Formatting

To maintain backwards compatibility with the library's plain console output, a custom OnclibFormatter was implemented. It suppresses metadata prefixes (timestamps, logger names, levels) for INFO level logs, while preserving full context for other levels.

Test Suite

The test suite's error handling was refactored to match the new error report behavior by introducing centralized err_400 and err_404 fixtures in tests/conftest.py.

Documentation

The final refactoring phase focused on using NumPy docstring style for consistent format

@kan-fu kan-fu requested a review from IanTBlack April 23, 2026 19:36
Comment thread src/onc/modules/_Messages.py Outdated
Set up a logger object for displaying verbose messages to console.

:param logger_name: The unique logger name to use. Can be shared between modules
:param level: The logging level to use. Default is 2, which corresponds to DEBUG.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Doc string should be updated to reflect logging numeric values.
https://docs.python.org/3/library/logging.html#logging-levels

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nevermind. I see that this is outdated.

@IanTBlack
Copy link
Copy Markdown
Collaborator

Received. I won't be able to begin reviewing and testing this for another week or so.

@kan-fu
Copy link
Copy Markdown
Collaborator Author

kan-fu commented Apr 24, 2026

No worries. Take your time. Besides the code review, I think it would be beneficial if you can test it using your actual script to see if any bug or unexpected behavior exists.

pip install onc@git+https://github.com/OceanNetworksCanada/api-python-client@issue-68-logging

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