Skip to content
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

AsyncioBackend: DeprecationWarning for StreamReader/StreamWriter in Py3.8 #256

Closed
florimondmanca opened this issue Aug 20, 2019 · 8 comments · Fixed by #369
Closed

AsyncioBackend: DeprecationWarning for StreamReader/StreamWriter in Py3.8 #256

florimondmanca opened this issue Aug 20, 2019 · 8 comments · Fixed by #369
Assignees

Comments

@florimondmanca
Copy link
Member

florimondmanca commented Aug 20, 2019

Python 3.8 builds are filled with warnings about asyncio API changes: https://travis-ci.org/encode/httpx/jobs/574559125

Looks like as as of Python 3.8, asyncio.open_connection() is deprecated in favor of asyncio.connect() — see the asyncio 3.8+ docs.

# < 3.8
reader, writer = await asyncio.open_connection('127.0.0.1', 8888)

# 3.8+
async with asyncio.connect('127.0.0.1', 8888) as stream:
    ...
# or:
stream = await asyncio.connect('127.0.0.1', 8888)

This is not a trivial API change, and I don't know if there are plans to back-port asyncio.connect() to 3.7 and below.

So at first glance resolving this would need to adapt the .connect() method on AsyncioBackend to use one or the other depending on the Python version we're running on — and also make Reader/Writer pair (and soon Stream) handle both possibilities ?

Originally raised in: #255 (comment)

@florimondmanca florimondmanca changed the title AsyncioBackend: DeprecationWarning for StreamReader/StreamWriter in Python 3.8 AsyncioBackend: DeprecationWarning for StreamReader/StreamWriter in Py3.8 Aug 20, 2019
@sethmlarson
Copy link
Contributor

So we're going to have to support both eventually, hmm.

@JayH5
Copy link
Member

JayH5 commented Sep 21, 2019

I can work on a compatibility layer of sorts for this ✋

@pquentin
Copy link
Contributor

It looks like asyncio is reverting the new streaming API: https://bugs.python.org/issue38242. Does that means the work from #369 is going to be irrelevant?

@florimondmanca
Copy link
Member Author

Oh wow, that’s interesting. (The decisions to revert in 3.8 is at the bottom of the thread.)

Looks like we’re going to have to revert too, indeed. :)

@florimondmanca
Copy link
Member Author

florimondmanca commented Sep 29, 2019

Let’s wait for this to be confirmed and merged on the Python side, then we can open a separate issue to tackle it.

Thanks a lot for detecting that @pquentin!

@JayH5
Copy link
Member

JayH5 commented Sep 29, 2019

Oh well 🙈 learnt some things along the way at least. Let's see when/if python/cpython#16455 is merged.

Thanks for the link @pquentin.

@aeros
Copy link

aeros commented Sep 29, 2019

I can confirm that we are removing the deprecation warnings for StreamReader, StreamWriter, StreamReaderProtocol, SubprocessProtocol, and Process in Python 3.8. They might be reinstated for 3.9 depending on what direction is taken with the asyncio streaming API, but they shouldn't be present in 3.8. This applies to any of the new functions added in python/cpython@23b4b69 (primary commit being reverted).

https://github.com/python/cpython/pull/16455/files is mostly just awaiting final review from the maintainers of asyncio. If Yury or Andrew have anything to add, they can directly edit the PR or open a new one if needed, but the new streaming API reversion will happen for Python 3.8 regardless of what happens with that PR.

@aeros
Copy link

aeros commented Sep 30, 2019

Just for clarification as to why 16455 was closed, Yury needed revert an additional component the streaming API, and we're due to finish everything up by Monday for the final tagging of 3.8.0rc1 (this is the final release blocker), so he opened up a separate PR. See python/cpython#16482 (comment) for more details.

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 a pull request may close this issue.

5 participants