bpo-36373: Deprecate explicit loop parameter in all public asyncio APIs [streams] by eamanu · Pull Request #13671 · python/cpython

eamanu · 2019-05-30T01:43:10Z

This PR deprecate explicit loop parameters in all public asyncio APIs

This issues is split to be easier to review.

Second step: streams.py

https://bugs.python.org/issue36373

This PR deprecate explicit loop parameters in all public asyncio APIs This issues is split to be easier to review. Second step: streams.py

asvetlov

Please remove all deprecations in public API but add these warnings to open_connection(), start_server(), open_unix_connection(), start_unix_server() instead.

Lib/asyncio/streams.py

asvetlov · 2019-05-30T10:56:02Z

Lib/asyncio/streams.py

Private API, people should never create StreamReader class explicitly but use await open_connection() and family.

asvetlov · 2019-05-30T10:56:12Z

Lib/asyncio/streams.py

Private API

bedevere-bot · 2019-05-30T10:57:34Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

eamanu · 2019-05-31T01:28:41Z

I have made the requested changes; please review again

bedevere-bot · 2019-05-31T01:28:44Z

Thanks for making the requested changes!

@asvetlov: please review the changes made to this pull request.

eamanu · 2019-05-31T01:29:20Z

@asvetlov the News Could be skipped?

asvetlov

Please run your PR by ./python -Weeror -m test -v test_asyncio.
I suspect that some tests will fail (but didn't check it myself).
These tests should be wrapped by with self.assertWarns(DeprecationWarning): ... construction

asvetlov · 2019-05-31T18:16:35Z

Lib/asyncio/streams.py

This class is actually private API. Please remove the warning, I pretty sure that asyncio factories like open_connection always send the loop. Double deprecation is redundant.

If so it should be documented as private, it does not appear to be : https://docs.python.org/3.6/library/asyncio-stream.html#streamreaderprotocol

And a quick search returns case of people passing the loop argument to it.
https://github.com/ssfdust/pyclewn/blob/555f2e5485f9938078adfe19388a0e1bb8af67f5/lib/clewn/tty.py#L119

Agree, it should be documented properly.
I've added _asyncio_internal flag.
Also, asincio.__init__ has __getattr__() now to raise DeprecationWarning for importing StreamReader, StreamWriter and StreamReaderProtocol.
We have an agreement with @1st1 to land asyncio streams code changes first and document them later before Release Candidate stage.

I supposed to make a PR for documentation. If somebody would help me -- I very appreciate this.
Especially taking into account that my English is rough :(
Usually Yuri replaces almost every my sentence on review :)

Hi @asvetlov I will help with the docs. :-)

bedevere-bot · 2019-05-31T18:19:14Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

asvetlov

please drop the check here

eamanu · 2019-06-05T01:36:36Z

I have made the requested changes; please review again

bedevere-bot · 2019-06-05T01:36:39Z

Thanks for making the requested changes!

@asvetlov: please review the changes made to this pull request.

eamanu · 2019-06-05T01:37:45Z

@asvetlov I run the ./python -Weeror -m test -v test_asyncio and the test are ok

Ran 2145 tests in 80.981s

OK (skipped=35)
test_asyncio passed in 1 min 21 sec

== Tests result: SUCCESS ==

1 test OK.

Total duration: 1 min 21 sec
Tests result: SUCCESS

miss-islington · 2019-06-05T05:45:54Z

Sorry, I can't merge this PR. Reason: Base branch was modified. Review and try the merge again..

miss-islington · 2019-06-05T05:45:56Z

Thanks @eamanu for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

bedevere-bot · 2019-06-05T05:46:21Z

GH-13833 is a backport of this pull request to the 3.8 branch.

asvetlov · 2019-06-05T05:49:07Z

Thank you @eamanu
Last days I was busy by firefighting with bugs to make 3.8 beta released.

The next big target is asyncio documentation cleanup (plus I need to land multithreaded support back again).

asvetlov · 2019-06-05T05:56:44Z

@eamanu would you take a look on asyncio/locks.py and asyncio/queues.py?

…Is [streams] (GH-13671) This PR deprecate explicit loop parameters in all public asyncio APIs This issues is split to be easier to review. Second step: streams.py https://bugs.python.org/issue36373 (cherry picked from commit 6d64a8f) Co-authored-by: Emmanuel Arias <emmanuelarias30@gmail.com>

eamanu · 2019-06-05T11:08:31Z

@eamanu would you take a look on asyncio/locks.py and asyncio/queues.py?

@asvetlov Yes yes, I will start work on that today. I was busy the last weekend. And I have pending #13670

asvetlov · 2019-06-05T13:46:08Z

Thank you. No rush, we have enough time during betas

…Is [streams] (pythonGH-13671) This PR deprecate explicit loop parameters in all public asyncio APIs This issues is split to be easier to review. Second step: streams.py https://bugs.python.org/issue36373

bpo-36373: Deprecate explicit loop parameter

ab1cdda

This PR deprecate explicit loop parameters in all public asyncio APIs This issues is split to be easier to review. Second step: streams.py

eamanu requested review from 1st1 and asvetlov as code owners May 30, 2019 01:43

the-knights-who-say-ni added the CLA signed label May 30, 2019

bedevere-bot added the awaiting review label May 30, 2019

asvetlov requested changes May 30, 2019

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting review labels May 30, 2019

Fix @asvetlov's comments.

153f6d0

bedevere-bot added awaiting change review and removed awaiting changes labels May 31, 2019

asvetlov requested changes May 31, 2019

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting change review labels May 31, 2019

asvetlov added the skip news label May 31, 2019

asvetlov requested changes May 31, 2019

View reviewed changes

Delete redundant warn message

54d7f43

bedevere-bot added awaiting change review and removed awaiting changes labels Jun 5, 2019

eamanu changed the title ~~bpo-36373: Deprecate explicit loop parameter in all public asyncio APIs~~ bpo-36373: Deprecate explicit loop parameter in all public asyncio APIs [streams] Jun 5, 2019

asvetlov approved these changes Jun 5, 2019

View reviewed changes

bedevere-bot removed the awaiting change review label Jun 5, 2019

bedevere-bot added the awaiting merge label Jun 5, 2019

asvetlov added 🤖 automerge labels Jun 5, 2019

miss-islington merged commit 6d64a8f into python:master Jun 5, 2019

bedevere-bot removed the awaiting merge label Jun 5, 2019

bedevere-bot removed the needs backport to 3.8 label Jun 5, 2019

eamanu deleted the bpo-36373-streams branch June 5, 2019 11:08

Uh oh!

Conversation

eamanu commented May 30, 2019 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asvetlov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented May 30, 2019

Uh oh!

eamanu commented May 31, 2019

Uh oh!

bedevere-bot commented May 31, 2019

Uh oh!

eamanu commented May 31, 2019

Uh oh!

asvetlov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented May 31, 2019

Uh oh!

asvetlov left a comment

Choose a reason for hiding this comment

Uh oh!

eamanu commented Jun 5, 2019

Uh oh!

bedevere-bot commented Jun 5, 2019

Uh oh!

eamanu commented Jun 5, 2019

Uh oh!

miss-islington commented Jun 5, 2019

Uh oh!

miss-islington commented Jun 5, 2019

Uh oh!

bedevere-bot commented Jun 5, 2019

Uh oh!

asvetlov commented Jun 5, 2019

Uh oh!

asvetlov commented Jun 5, 2019

Uh oh!

eamanu commented Jun 5, 2019

Uh oh!

asvetlov commented Jun 5, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Comments

eamanu commented May 30, 2019 •

edited by bedevere-bot

Loading