Skip to content

Comments

fix: don't use module-level logging methods#46

Merged
WSH032 merged 7 commits intoWSH032:mainfrom
dvarrazzo:fix-logger
Jan 15, 2025
Merged

fix: don't use module-level logging methods#46
WSH032 merged 7 commits intoWSH032:mainfrom
dvarrazzo:fix-logger

Conversation

@dvarrazzo
Copy link
Contributor

@dvarrazzo dvarrazzo commented Jan 13, 2025

Calling these methods might add handlers to the root logger, which can interfere with applications that don't use it.

Fix #45.

Checklist

  • I've read CONTRIBUTING.md.
  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.

Calling these methods might add handlers to the root logger, which can
interfere with applications that don't use it.

Fix WSH032#45.
@WSH032
Copy link
Owner

WSH032 commented Jan 13, 2025

Thanks, LGTM.

Would you mind making them private?

_logger = logging.getLogger(__name__)

The new version httpx deprecated and removed some APIs, causing the CI to fail. Give me some time, I will fix it.

@codecov
Copy link

codecov bot commented Jan 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.41%. Comparing base (558a86b) to head (b6a21ca).
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #46      +/-   ##
==========================================
+ Coverage   96.96%   97.41%   +0.45%     
==========================================
  Files           9        9              
  Lines         461      464       +3     
  Branches       44       35       -9     
==========================================
+ Hits          447      452       +5     
+ Misses          9        7       -2     
  Partials        5        5              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@WSH032
Copy link
Owner

WSH032 commented Jan 14, 2025

The test for python3.8 on Ubuntu failed due to frankie567/httpx-ws#29. I have rerun the workflow multiple times in another PR and almost 100% reproduced this issue.

Except for reporting this issue to httpx-ws, there is nothing we can do.

If you are ready, I can use admin privileges to bypass this check. After all, it only fails in 3.8, and httpx-ws has already dropped support for 3.8.


EDIT:

I just remembered that this bug was fixed in httpx-ws >= 0.7, but httpx-ws dropped support for python3.8 in 0.7.
That's why it only fails for python3.8.

So I am going to drop support for python3.8 and bump httpx-ws >= 0.7.

I call it a day. I will do it tomorrow.

@WSH032 WSH032 changed the title Don't use module-level logging methods fix: don't use module-level logging methods Jan 14, 2025
@WSH032
Copy link
Owner

WSH032 commented Jan 14, 2025

CI passed, do you have any other changes?

@WSH032 WSH032 merged commit 1807434 into WSH032:main Jan 15, 2025
19 checks passed
@dvarrazzo dvarrazzo deleted the fix-logger branch December 8, 2025 00:21
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.

fastapi-proxy-lib causes logging duplication

2 participants