Remove requests, use httpx for sync and async HTTP requests#876
Remove requests, use httpx for sync and async HTTP requests#876Marc-Andrieu wants to merge 3 commits intomainfrom
requests, use httpx for sync and async HTTP requests#876Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #876 +/- ##
=======================================
Coverage 84.43% 84.43%
=======================================
Files 206 206
Lines 14918 14918
=======================================
Hits 12596 12596
Misses 2322 2322 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| try: | ||
| self.matrix.send_message(self.room_id, msg) | ||
| self.background_tasks.add_task( |
There was a problem hiding this comment.
Loggers are encapsulated in a QueueHandlers which send logs in a dedicated thread. I'm honstly unsure about the consequences of using FastAPI BackgroundTask here.
I believe:
- it should be unneeded as logs are already sent in their own thread
- depending on BackgroundTask implementation, it could pull back the log emission in the main thread, which would be counterproductive
- the execution of the task would be planned by FastAPI, at an unknown instant. We have no guarantee that a task added outside endpoint executions would be instantly scheduled.
I these fears are unfounded, I think it would be a good thing to add comments in the code explaining the benefits of using a BackgroundTask here
There was a problem hiding this comment.
You're right... Even the native logging HTTPHandler (https://docs.python.org/3/library/logging.handlers.html#httphandler) seems synchronous...
After all I don't care much about the "sync vs async" debate per se, I just want:
- to remove
requestsandtype_requestsfrom our dependencies - not to alter the current performance
There was a problem hiding this comment.
Update: this PR is about to become a LOT easier!
The very 1st sentence of https://www.python-httpx.org/ states that httpx "provides sync and async APIs"
requests and support async loggingrequests, use httpx for sync and async HTTP requests
requests, use httpx for sync and async HTTP requestsrequests, use httpx for sync and async HTTP requests
Description
Summary
Split #845 into a 3rd PR with only the removal of
requestsand its major consequence: send logs asynchronously.Changes Made
requestsuse withhttpx, or remove itBackgroundTasksType of Change
Impact & Scope
Testing
Documentation