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

bpo-46829: Deprecate passing a message into Future.cancel() and Task.cancel() #31840

Merged
merged 6 commits into from
Mar 23, 2022

Conversation

asvetlov
Copy link
Contributor

@asvetlov asvetlov commented Mar 12, 2022

@gvanrossum
Copy link
Member

gvanrossum commented Mar 12, 2022

Please hold off on merging this until we've come to some kind of agreement on whether to deprecate .cancel(msg). See bpo-46829.

@gvanrossum
Copy link
Member

Also, if we do this we should add deprecation markup to the docs. Search for 'method:: cancel' in asyncio-future.rst and asyncio-task.rst. Also there's a note at the very bottom of asyncio-future.rst about the difference between asyncio.Future.cancel and concurrent.futures.Future.cancel that should be deprecated.

@asvetlov
Copy link
Contributor Author

Forgot about docs, thanks.
Sure, I'll wait for the final decision.
Just wanted to don't do it at the last moment before the feature freeze.

@asvetlov
Copy link
Contributor Author

asvetlov commented Mar 13, 2022

I've updated the doc.

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

I don't know how long to wait for @cjerdonek to respond or agree.

@asvetlov
Copy link
Contributor Author

@cjerdonek would you comment on this pull request, please?

@gvanrossum
Copy link
Member

I'm just going to merge this. @cjerdonek, if you have a convincing argument to keep this, we can always revert this PR.

@gvanrossum gvanrossum merged commit 0360e9f into main Mar 23, 2022
@gvanrossum gvanrossum deleted the deprecate-cancel-msg branch March 23, 2022 15:43
@gvanrossum
Copy link
Member

At the core dev sprint we discussed this and decided to revert this. I'm aiming to get the reversal into 3.11.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants