gh-103607: Fixing pause_reading called in connection made is ignored by Itayazolay · Pull Request #17425 · python/cpython

Itayazolay · 2019-12-01T12:54:21Z

bpo-31821: Fixing pause_reading called in connection made is ignored

https://bugs.python.org/issue31821

Issue: Cannot pause asyncio transport immediately #103607

added is_reading() for unix transports

… into fix-issue-31821

csabella · 2019-12-15T12:53:48Z

This is an alternative solution to #4053.

pitrou · 2019-12-15T13:06:00Z

I'm not active on asyncio anymore. Perhaps @1st1 or @asvetlov can take a look. But I can still spot a problem: this PR only addresses the selector-based event loops, therefore it is incomplete.

Itayazolay · 2020-01-10T14:31:54Z

Thanks for your response,
I included unix_events in this PR, but I don't think the ssl_events are relevant to this PR since this bug only included protocol classes that are paused on connection_made, and either way SSL is performing the handshake when a connection is made.
I also added is_reading() to unixReadPipeTransport, since it inherits from ReadTransport but isn't implemented.
As for PR #4053, I see that it's inactive since January 2019.
Please let me know if there is something I need to do further.

Lib/asyncio/selector_events.py

bedevere-bot · 2020-01-12T13:05:09Z

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.

…ort parent class

Itayazolay · 2020-01-12T16:47:36Z

I have made the requested changes; please review again

bedevere-bot · 2020-01-12T16:47:39Z

Thanks for making the requested changes!

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

asvetlov

Aha, I see the point.
pause_reading belongs to ReadTransport, _SelectorTransport is used by _SelectorDatagramTransport also.
I know, we have a little mess in transport inheritance currently; it should be fixed.

Perhaps the previous version was better; I'd like to clean up the inheritance before returning to the review of this PR anyway.
Sorry for misleading.

bedevere-bot · 2020-01-12T18:42:20Z

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.

gvanrossum

Good work. I would like to see one more test. Also, @kumaraditya303 do you have any comments?

Lib/test/test_asyncio/test_selector_events.py

kumaraditya303 · 2023-04-20T14:49:36Z

I'll review this by tomorrow.

kumaraditya303

Please add a test for proactor event loop too.

add Proactor events tests

Itayazolay · 2023-04-22T16:10:24Z

Thank you both for responding!
I've refactored the test to be called from within the connection_made, and added the test to the proactor loop as well.
Is there anything else I should do?

Itayazolay · 2023-04-25T10:07:14Z

I have made the requested changes; please review again

bedevere-bot · 2023-04-25T10:07:19Z

Thanks for making the requested changes!

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

outdated

kumaraditya303

LGTM, thanks!

test added

gvanrossum · 2023-04-26T23:00:17Z

@kumaraditya303 Feel free to merge if you are happy now.

miss-islington · 2023-04-27T10:03:33Z

Thanks @Itayazolay for the PR, and @kumaraditya303 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

…tion_made` in `asyncio`. (pythonGH-17425) (cherry picked from commit 78942ec) Co-authored-by: Itayazolay <itayazolay@gmail.com> Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>

bedevere-bot · 2023-04-27T10:03:42Z

GH-103918 is a backport of this pull request to the 3.11 branch.

…ction_made` in `asyncio`. (GH-17425) (#103918) gh-103607: Fix `pause_reading` to work when called from `connection_made` in `asyncio`. (GH-17425) (cherry picked from commit 78942ec) Co-authored-by: Itayazolay <itayazolay@gmail.com> Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>

Itayazolay added 2 commits December 1, 2019 13:12

Fixing pause_reading called in connection made is ignored

2be906d

added is_reading() for unix transports

Revert changes to unix_events

8b1f17b

Itayazolay requested review from 1st1 and asvetlov as code owners December 1, 2019 12:54

the-knights-who-say-ni added the CLA signed label Dec 1, 2019

bedevere-bot added the awaiting review label Dec 1, 2019

blurb-it bot and others added 4 commits December 1, 2019 12:58

📜🤖 Added by blurb_it.

9db5d28

fix _add_reader in SelectorTransport

92d8ca7

Merge branch 'fix-issue-31821' of https://github.com/Itayazolay/cpython…

780970d

… into fix-issue-31821

fix tests

7acf410

csabella requested a review from pitrou December 15, 2019 12:54

Itayazolay added 2 commits January 9, 2020 22:45

Added functionality to unix_events

a16f4d3

fix tests and added is_reading() to unixReadPipeTransport

c4b3444

asvetlov requested changes Jan 12, 2020

View reviewed changes

Lib/asyncio/selector_events.py Outdated Show resolved Hide resolved

bedevere-bot removed the awaiting review label Jan 12, 2020

bedevere-bot added the awaiting changes label Jan 12, 2020

Move ReadTransport SelectorSocketTransport override to SelectorTransp…

9da5fce

…ort parent class

bedevere-bot added awaiting change review and removed awaiting changes labels Jan 12, 2020

bedevere-bot requested a review from asvetlov January 12, 2020 16:47

asvetlov previously requested changes Jan 12, 2020

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting change review labels Jan 12, 2020

gvanrossum previously requested changes Apr 20, 2023

View reviewed changes

Lib/test/test_asyncio/test_selector_events.py Show resolved Hide resolved

kumaraditya303 unassigned asvetlov Apr 20, 2023

kumaraditya303 removed the request for review from pitrou April 20, 2023 14:49

kumaraditya303 self-assigned this Apr 20, 2023

kumaraditya303 reviewed Apr 21, 2023

View reviewed changes

Refactor pause reading to be called from within connection made

dbbeedd

add Proactor events tests

Itayazolay requested review from asvetlov and gvanrossum April 23, 2023 08:35

bedevere-bot added awaiting change review and removed awaiting changes labels Apr 25, 2023

fixup news

17d036b

kumaraditya303 approved these changes Apr 25, 2023

View reviewed changes

kumaraditya303 added needs backport to 3.11 only security fixes and removed awaiting change review labels Apr 25, 2023

willingc added the awaiting merge label Apr 26, 2023

kumaraditya303 merged commit 78942ec into python:main Apr 27, 2023

bedevere-bot removed the awaiting merge label Apr 27, 2023

bedevere-bot removed the needs backport to 3.11 only security fixes label Apr 27, 2023

zhengfeihe mentioned this pull request Apr 15, 2024

Implement unix read_pipe.is_reading() method #82495

Closed

Uh oh!

Conversation

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

Uh oh!

Uh oh!

csabella commented Dec 15, 2019

Uh oh!

pitrou commented Dec 15, 2019

Uh oh!

Itayazolay commented Jan 10, 2020

Uh oh!

Uh oh!

bedevere-bot commented Jan 12, 2020

Uh oh!

Itayazolay commented Jan 12, 2020

Uh oh!

bedevere-bot commented Jan 12, 2020

Uh oh!

asvetlov left a comment

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Jan 12, 2020

Uh oh!

gvanrossum left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kumaraditya303 commented Apr 20, 2023

Uh oh!

kumaraditya303 left a comment

Choose a reason for hiding this comment

Uh oh!

Itayazolay commented Apr 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Itayazolay commented Apr 25, 2023

Uh oh!

bedevere-bot commented Apr 25, 2023

Uh oh!

kumaraditya303 left a comment

Choose a reason for hiding this comment

Uh oh!

gvanrossum commented Apr 26, 2023

Uh oh!

miss-islington commented Apr 27, 2023

Uh oh!

bedevere-bot commented Apr 27, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

Itayazolay commented Dec 1, 2019 •

edited by bedevere-bot

Loading

Itayazolay commented Apr 22, 2023 •

edited

Loading