-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Support Trio with httpx #3089
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?
Support Trio with httpx #3089
Conversation
🔍 Preview links for changed docs |
|
||
|
||
async def _sleep(seconds: float) -> None: | ||
if sniffio.current_async_library() == "trio": |
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.
Would it make sense to ask the transport what the current library is, since it stores it? Not super important, but it feels unnecessary to constantly run the detection logic in sniffio.
dependencies = [ | ||
"elastic-transport>=9.1.0,<10", | ||
# TODO revert before merging/releasing | ||
"elastic-transport @ git+https://github.com/pquentin/elastic-transport-python.git@trio-support", |
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.
Adding a comment here just to remind you that need this to be edited. (not sure if your TODO
comment triggers some alert or linter...)
if not hasattr(elasticsearch, "AsyncElasticsearch"): | ||
pytest.skip("test requires 'AsyncElasticsearch' and aiohttp to be installed") | ||
|
||
print("async!", elasticsearch_url) |
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.
debug print?
from ...utils import parse_version | ||
|
||
# We're not using `pytest.mark.anyio` here because it would run the test suite twice, | ||
# which does not work as it does not fully clean up after itself. |
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.
Probably something that we'll want to address at some point.
event_loop = asyncio.get_running_loop() | ||
async def test_sniff_on_start_close_unlocks_async_calls(self, anyio_backend): | ||
if anyio_backend == "trio": | ||
pytest.skip("trio does not support sniffing") |
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.
I think it isn't necessary to skip here, or even use the anyio
fixture and functions, since this whole class is marked with asyncio
and not anyio
.
"sed", | ||
"-i.bak", | ||
"s/pytest.mark.asyncio/pytest.mark.sync/", | ||
"s/pytest.mark.anyio/pytest.mark.sync/", |
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.
I think this should handle both anyio
and asyncio
markers, just in case. Unless we ignore the asyncio marker on purpose just so that the build fails when we mistakenly use it instead of anyio?
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.
Some minor/optional comments and suggestions, but overall LGTM.
No description provided.