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.

Sign up for GitHub

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

Jump to bottom

bpo-22087: Fix Policy.get_event_loop() to detect fork #7208

Merged
merged 2 commits into from
May 29, 2018
Merged

bpo-22087: Fix Policy.get_event_loop() to detect fork #7208

merged 2 commits into from
May 29, 2018

Conversation

1st1
Copy link
Member

@1st1 1st1 commented May 29, 2018 •
edited by bedevere-bot
Loading

Original patch by Dan O'Reilly.

https://bugs.python.org/issue22087

@1st1 1st1 requested a review from asvetlov as a code owner May 29, 2018 16:08
@pitrou
Copy link
Member

pitrou commented May 29, 2018

I think it would be nice if the test did a bit more, for example run some coroutine or perhaps even some simple socket ping-pong. That would check that other infrastructure is also fork-safe, not just the event loop instantiation logic.

@1st1
Copy link
Member Author

1st1 commented May 29, 2018

Sure, added a basic test for that. Don't think we need more than that though, as the entirety of "other infrastructure" should be initialized via the loop's constructor.

pitrou
pitrou approved these changes May 29, 2018
View reviewed changes
Copy link
Member

@pitrou pitrou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I'm assuming the test isn't run on Windows?

@1st1
Copy link
Member Author

1st1 commented May 29, 2018

I'm assuming the test isn't run on Windows?

Yes, it's only for Unix.

asvetlov
asvetlov approved these changes May 29, 2018
View reviewed changes
Copy link
Contributor

@asvetlov asvetlov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@1st1 1st1 merged commit 5d97b7b into python:master May 29, 2018
@miss-islington
Copy link
Contributor

Thanks @1st1 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7.
🐍🍒⛏🤖

@1st1 1st1 deleted the fixfork branch May 29, 2018 19:38
@bedevere-bot
Copy link

GH-7215 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 29, 2018
@1st1 @miss-islington
bpo-22087: Fix Policy.get_event_loop() to detect fork (pythonGH-7208)
3969a51 
Original patch by Dan O'Reilly.
(cherry picked from commit 5d97b7b)

Co-authored-by: Yury Selivanov <[email protected]>
@miss-islington
Copy link
Contributor

Sorry, @1st1, I could not cleanly backport this to 3.6 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 5d97b7bcc19496617bf8c448d2f149cc28c73bc7 3.6

@bedevere-bot
Copy link

GH-7218 is a backport of this pull request to the 3.6 branch.

miss-islington added a commit that referenced this pull request May 29, 2018
@miss-islington @1st1
bpo-22087: Fix Policy.get_event_loop() to detect fork (GH-7208)
2a7eb0b 
Original patch by Dan O'Reilly.
(cherry picked from commit 5d97b7b)

Co-authored-by: Yury Selivanov <[email protected]>
1st1 added a commit to 1st1/cpython that referenced this pull request May 29, 2018
@1st1
bpo-22087: Fix Policy.get_event_loop() to detect fork (pythonGH-7208)
e8dfb2c 
Original patch by Dan O'Reilly.
1st1 added a commit to 1st1/cpython that referenced this pull request May 29, 2018
@1st1
Revert "bpo-22087: Fix Policy.get_event_loop() to detect fork (python…
c3ac9f5 
…GH-7208)"

This reverts commit 5d97b7b.
1st1 added a commit to 1st1/cpython that referenced this pull request May 29, 2018
@1st1
Revert "bpo-22087: Fix Policy.get_event_loop() to detect fork (python…
2dd522e 
…GH-7208)"

This reverts commit 2a7eb0b.
1st1 added a commit that referenced this pull request May 30, 2018
@1st1
Revert "bpo-22087: Fix Policy.get_event_loop() to detect fork (GH-7208)…
99279ad 
…" (#7232)

This reverts commit 5d97b7b.
1st1 added a commit that referenced this pull request May 30, 2018
@1st1
Revert "bpo-22087: Fix Policy.get_event_loop() to detect fork (GH-7208)…
af9cda9 
…" (GH-7233)

This reverts commit 2a7eb0b.
@danoreilly danoreilly mannequin mentioned this pull request Apr 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asvetlov asvetlov asvetlov approved these changes

@pitrou pitrou pitrou approved these changes

Assignees

@1st1 1st1

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@1st1 @pitrou @miss-islington @bedevere-bot @asvetlov @the-knights-who-say-ni