Skip to content

Add externalPythonDependencies logging category#18067

Merged
SaschaCowley merged 4 commits into
nvaccess:masterfrom
LeonarddeR:dependencyLogging
May 13, 2025
Merged

Add externalPythonDependencies logging category#18067
SaschaCowley merged 4 commits into
nvaccess:masterfrom
LeonarddeR:dependencyLogging

Conversation

@LeonarddeR

Copy link
Copy Markdown
Collaborator

Link to issue number:

Follow up for #10393

Summary of the issue:

Since #10393, we log at level warning or higher for external Python dependencies. There is no way to enable logging at lower levels, such as debug. This can however be pretty helpful for external dependencies like comtypes to see what's going on under the hood.

Description of user facing changes

Have extra logging when a new debugging category, externalPythonDependencies, is enabled.

Description of development approach

The overall logging level is now determined from the level of the root logger, which is no longer limited to a minimum of WARNING. Instead, a filter is implemented on the handler that filters appropriately. Note that initially, it made sense to add this filter to the root logger, but filters don't propagate from the root logger to all loggers below.

Testing strategy:

Calling comtypes.CoInitialize with and without the new category enabled, logging at level debug. Ensured that behavior was as expected (i.e. comtypes logged when the category was enabled)

Known issues with pull request:

In #10393, I mentioned issues with re-entrancy when comtypes logs at level debug. I have not yet reproduced this. That said, the logging categories are documented in such a way that they should only be enabled on request. I don't think this needs extra documentation.

Code Review Checklist:

  • Documentation:
    • Change log entry
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • API is compatible with existing add-ons.
  • Security precautions taken.

@coderabbitai summary

@LeonarddeR LeonarddeR requested a review from a team as a code owner May 7, 2025 20:41
@LeonarddeR LeonarddeR requested a review from SaschaCowley May 7, 2025 20:41
@SaschaCowley SaschaCowley added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label May 12, 2025

@SaschaCowley SaschaCowley left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, @LeonarddeR

@SaschaCowley SaschaCowley merged commit 9a8380e into nvaccess:master May 13, 2025
15 checks passed
@github-actions github-actions Bot added this to the 2025.2 milestone May 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants