-
Notifications
You must be signed in to change notification settings - Fork 5
Migrate to pyqwest #88
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| cleanup.push_async_exit(transport.__aexit__) | ||
| http_client = Client(transport) | ||
| else: | ||
| http_client = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added coverage of a client without an explicit transport which could have been good before too
|
|
||
|
|
||
| # Convert a timeout with connect semantics to a httpx.Timeout. Connect timeouts | ||
| # should apply to an entire operation but this is difficult in synchronous Python code |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With reqwest, the timeout applies from start to end of response read so it works without any weirdness
| final_count = counter["requests"] | ||
|
|
||
| app = HaberdasherASGIApplication(counting_haberdasher()) | ||
| # Use uvicorn since it supports lifespan |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pyqwest ASGITransport supports lifespan
| assert response.headers == response_headers | ||
|
|
||
|
|
||
| # To exercise timeouts, can't use mock transports |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mock transports support timeouts
| """Close the HTTP client. After closing, the client cannot be used to make requests.""" | ||
| if not self._closed: | ||
| self._closed = True | ||
| if self._close_client: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's nothing left to close. I left he methods though in case we need to add in the future
Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
6c6c276 to
b41712a
Compare
As discussed, this is the spike to replace httpx with pyqwest. Seems like a lot to like here, but of course I'm biased :) Happy to hear what others think.