GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-94732: Fix KeyboardInterrupt race in asyncio run_forever() by hetmankp · Pull Request #94733 · python/cpython

Conversation

@hetmankp

@hetmankp hetmankp commented Jul 11, 2022 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

This addresses issue #94732, ensuring that the event loop's _thread_id
attribute and the asyncgen hooks set by sys.set_asyncgen_hooks() are
always restored no matter where a KeyboardInterrupt exception is raised.

  • Issue: Race with KeyboardInterrupt in asyncio loop run_forever() method #94732

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@hetmankp
pythongh-94732: Fix KeyboardInterrupt race in asyncio run_forever()
5fa35ec
This addresses issue python#94732, ensuring that the event loop's _thread_id
attribute and the asyncgen hooks set by sys.set_asyncgen_hooks() are
always restored no matter where a KeyboardInterrupt exception is raised.
@hetmankp
hetmankp requested review from 1st1 and asvetlov as code owners July 11, 2022 06:22
@ghost

ghost commented Jul 11, 2022 •
edited by ghost
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

The following commit authors need to sign the Contributor License Agreement:

  • [email protected]

Click the button to sign:
CLA not signed

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot added the awaiting review label Jul 11, 2022
@bedevere-bot

bedevere-bot commented Jul 11, 2022

Copy link
Copy Markdown

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gvanrossum
gvanrossum approved these changes Sep 4, 2022
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting review labels Sep 4, 2022
@gvanrossum

gvanrossum commented Sep 4, 2022

Copy link
Copy Markdown
Member

I am okay with skipping news, it please do sign the CLA.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hetmankp

hetmankp commented Sep 5, 2022

Copy link
Copy Markdown
Contributor Author

I am okay with skipping news, it please do sign the CLA.

I have attempted to sign the CLA (i.e. enable the app) but I have the "Keep my email addresses private" option selected on Github, and the CLA app doesn't seem to accept my Github noreply email address. (My Github account is old enough, the noreply address shouldn't need the ID number).

Is this form of signing the agreement not supported at all or did I do something wrong?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gvanrossum

gvanrossum commented Sep 5, 2022

Copy link
Copy Markdown
Member

Maybe @ambv can help with the CLA issue?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ambv

ambv commented Sep 5, 2022

Copy link
Copy Markdown
Contributor

the CLA app doesn't seem to accept my Github noreply email address

@hetmankp, can you say what exactly seems to be happening? We do have handling for the old-style noreply addresses, too, maybe there's a bug in there.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@AlexWaygood AlexWaygood added the skip news label Sep 5, 2022
@hetmankp

hetmankp commented Sep 6, 2022

Copy link
Copy Markdown
Contributor Author

@hetmankp, can you say what exactly seems to be happening? We do have handling for the old-style noreply addresses, too, maybe there's a bug in there.

Sorry @ambv , I should have been more specific. When I click the "SIGN IN WITH GITHUB TO AGREE" button on the The PSF Individual Contributor Agreement page, I get taken to a white page with one line of text which reads:

Thank you for authorizing our application, but the CLA must be signed by the users who contributed to the PR. Authors emails are: [email protected].

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gvanrossum
Merge branch 'main' into pythongh-94732
c95d0ff
@gvanrossum gvanrossum removed the skip news label Sep 14, 2022
@bedevere-bot

bedevere-bot commented Sep 14, 2022

Copy link
Copy Markdown

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

1 similar comment
@bedevere-bot

bedevere-bot commented Sep 14, 2022

Copy link
Copy Markdown

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gvanrossum gvanrossum added the skip news label Sep 14, 2022
@gvanrossum

gvanrossum commented Sep 14, 2022

Copy link
Copy Markdown
Member

How's the CLA signing going? Still no resolution?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gvanrossum

gvanrossum commented Sep 29, 2022

Copy link
Copy Markdown
Member

@hetmankp I cannot merge this without a signed CLA, alas. @ambv What can we do?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hetmankp

hetmankp commented Oct 3, 2022

Copy link
Copy Markdown
Contributor Author

@hetmankp I cannot merge this without a signed CLA, alas. @ambv What can we do?

@gvanrossum @ambv , more than happy to work with whoever is available to provide further debugging information from my end.

Obviously this commit is simple enough we can probably find an alternative way forward if it comes to that, but if at all possible I'd rather we address the licencing system bug since it could affect other committers as well (plus it is the second time it has caused an issue for me attempting to submit a merge request into CPython).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gvanrossum

gvanrossum commented Oct 3, 2022

Copy link
Copy Markdown
Member

I don't know what's going on in the CLA bot, but presumably it would be avoidable if you linked a proper email address to your GitHub account and created a new PR using that email? (IIRC you can set the email address when committing with a git flag, or you can change your git config to set it.)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hetmankp

hetmankp commented Oct 3, 2022

Copy link
Copy Markdown
Contributor Author

I don't know what's going on in the CLA bot, but presumably it would be avoidable if you linked a proper email address to your GitHub account and created a new PR using that email? (IIRC you can set the email address when committing with a git flag, or you can change your git config to set it.)

@gvanrossum I'd prefer not to do that though I think the issue might have been with my GitHub account anyway. It's an old account so as far as I'm aware it should have been using the simple anonymous email format without the ID number. However, I've just opened up a new pull request with the aforementioned ID number included and the CLA signing bot seems happy with that one.

The new pull request with the CLA signed is #97765 , so I think this one can be closed and that one can be used instead. Let me know if you'd like this done some other way though. Thanks and sorry for the complications.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gvanrossum gvanrossum closed this Oct 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gvanrossum gvanrossum gvanrossum approved these changes
@1st1 1st1 Awaiting requested review from 1st1 1st1 is a code owner
@asvetlov asvetlov Awaiting requested review from asvetlov asvetlov is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting merge skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@hetmankp @bedevere-bot @gvanrossum @ambv @AlexWaygood
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.