Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[3.13] gh-128308: Revert "pass**kwargs to asyncio task_factory (GH-128768) (#130084)"#133808

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
kumaraditya303 wants to merge2 commits intopython:3.13fromkumaraditya303:revert

Conversation

kumaraditya303
Copy link
Contributor

@kumaraditya303kumaraditya303 commentedMay 10, 2025
edited by github-actionsbot
Loading

@kumaraditya303kumaraditya303 changed the titleRevert "[3.13] gh-128308: pass**kwargs to asyncio task_factory (GH-128768) (#130084)"[3.13] gh-128308: Revert pass**kwargs to asyncio task_factory (GH-128768) (#130084)"May 10, 2025
Copy link
Member

@gvanrossumgvanrossum left a comment

Choose a reason for hiding this comment

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

Only the backport is being reverted right? Please explain in the commit message why.

@gvanrossum
Copy link
Member

Does this spell doom for the 3.14 change????@graingert

@graingert
Copy link
Contributor

graingert commentedMay 10, 2025
edited
Loading

I don't think so, a new kwargcontext was addedand passed in asyncio.Runner in 3.11 so there's precedent for this sort of change. Foreager_start we're adding it but not passing it anywhere so it's not breaking any existing code. Forname it's being passed and added.

If we decide passingname is breaking (and passingcontext was also breaking and we agree it shouldn't have been added) we can popname from the kwargs inBaseEventLoop.create_task and use a flag and deprecation cycle to add it back like@serhiy-storchaka suggested. The feature for the eager_start is still valid

@picnixzpicnixz changed the title[3.13] gh-128308: Revert pass**kwargs to asyncio task_factory (GH-128768) (#130084)"[3.13] gh-128308: Revert "pass**kwargs to asyncio task_factory (GH-128768) (#130084)"May 10, 2025
@gvanrossum
Copy link
Member

So it just shouldn’t have been backported. Sorry.

somehow I can’t find the issue suggesting the revert.

@gvanrossum
Copy link
Member

gvanrossum commentedMay 10, 2025
edited
Loading

See#133745. That argument applies to 3.14 too.In fact we broke the “name, context may be positional” for create task?

@graingert
Copy link
Contributor

graingert commentedMay 10, 2025
edited
Loading

Ah yes definitely! I'll get a PR for 3.14 to fix the positional behavior for name and context ah no they were keyword only

@serhiy-storchaka
Copy link
Member

name andcontext always were keyword only.

But we broke explicitcontext=None.create_task(coro, context=None) did not passcontext to the task factory.

gvanrossum reacted with thumbs up emoji

@pablogsal
Copy link
Member

pablogsal commentedMay 10, 2025
edited
Loading

Notice that several people have implemented workarounds since then so we need to ensure we preserve both behaviors to not break the people that have adapted. For example:beeware/toga#3395

@gvanrossum
Copy link
Member

Looks like we have more work to do -- 3.13.4 must support the workarounds that people have already made for 3.13.3.

@alicederyn
Copy link

FWIW as one of the people who made a workaround, we just followed the documentation and passed through**kwargs, which is compatible with a simple revert;beeware/toga have also done it this way. As well as being the new documented contract, this is also the simplest way to remain compatible with Python 3.12 and earlier, so hopefully most people will have done the same.

@gvanrossum
Copy link
Member

Thanks@alicederyn, that's very helpful.

alicederyn reacted with heart emoji

@gvanrossum
Copy link
Member

@graingert Are you going to update this PR to not break the workaround described by@alicederyn ?

@graingert
Copy link
Contributor

Does a plain revert break the workaround?

@gvanrossum
Copy link
Member

gvanrossum commentedMay 12, 2025
edited
Loading

Does a plain revert break the workaround?

I think so. Pre-revert (but only in 3.13.3), you could have a "private" keyword parameter passed to create_task() that's passed on to your factory (assuming you have control over the factory and your factory accepts this private keyword). So I think the various create_task() methods should (in addition to name= and context=) grow a new**kwargs that just is passed on to the factory or task (causing an error if it's not expected).

It's possible that that fix also models the improved version for 3.14b2 (which should pop name andcancel context if they're None, IIUC).

@graingert
Copy link
Contributor

graingert commentedMay 12, 2025
edited
Loading

Ah right, that's a better option!

I'm not sure about this:

and cancel if they're None, IIUC).

Do you meanTask.cancel(), or abort withRuntimeError, orcoro.close(), or something else?

@gvanrossum
Copy link
Member

Sorry, "and cancel" should have been "and context". Will fix in the original.

@graingert
Copy link
Contributor

Do you want to always pop name in 3.13.4?

@gvanrossum
Copy link
Member

So, passing name= through kwargs to the factory must be a feature that factories must support to be compatible with 3.13.3 (but not later versions of 3.13), and to be compatible with 3.14b1+. 3.13.4+ must revert to the lame setname call to send the factory the name (a very old hack to be compatible with earlier versions of uvloop IIRC), but for 3.14b2 and everything after we require factories to support passing name= via **kwargs, and won't do the setname call any more. Right?

@gvanrossum
Copy link
Member

Do you want to always pop name in 3.13.4?

I think so, since pre-3.13.3 factories may not support any keywords (other than and empty dict) and IIUC there's no way to tell if a factory supports that or requires setname, and I don't want to introduce a new API so we could ask a task factory about this.

@graingert
Copy link
Contributor

Ok I opened up a draft here:#133948

the news fragment is going to be fun, I've left it blank for now

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@gvanrossumgvanrossumgvanrossum approved these changes

@1st11st1Awaiting requested review from 1st11st1 is a code owner

@asvetlovasvetlovAwaiting requested review from asvetlovasvetlov is a code owner

@willingcwillingcAwaiting requested review from willingcwillingc is a code owner

@Yhg1sYhg1sAwaiting requested review from Yhg1s

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

6 participants
@kumaraditya303@gvanrossum@graingert@serhiy-storchaka@pablogsal@alicederyn

[8]ページ先頭

©2009-2025 Movatter.jp