| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
…ry (pythonGH-128768) (python#130084)" This reverts commit 7b0543e.
There was a problem hiding this comment.
Only the backport is being reverted right? Please explain in the commit message why.
Sorry, something went wrong.
|
Does this spell doom for the 3.14 change???? @graingert |
Sorry, something went wrong.
|
I don't think so, a new kwarg context was added and passed in asyncio.Runner in 3.11 so there's precedent for this sort of change. For eager_start we're adding it but not passing it anywhere so it's not breaking any existing code. For name it's being passed and added. If we decide passing name is breaking (and passing context was also breaking and we agree it shouldn't have been added) we can pop name from the kwargs in BaseEventLoop.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 |
Sorry, something went wrong.
|
So it just shouldn’t have been backported. Sorry. somehow I can’t find the issue suggesting the revert. |
Sorry, something went wrong.
|
See #133745. That argument applies to 3.14 too. In fact we broke the “name, context may be positional” for create task? |
Sorry, something went wrong.
|
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 |
Sorry, something went wrong.
|
name and context always were keyword only. But we broke explicit context=None. create_task(coro, context=None) did not pass context to the task factory. |
Sorry, something went wrong.
|
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 |
Sorry, something went wrong.
|
Looks like we have more work to do -- 3.13.4 must support the workarounds that people have already made for 3.13.3. |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
|
Thanks @alicederyn, that's very helpful. |
Sorry, something went wrong.
|
@graingert Are you going to update this PR to not break the workaround described by @alicederyn ? |
Sorry, something went wrong.
|
Does a plain revert break the workaround? |
Sorry, something went wrong.
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 and cancel context if they're None, IIUC). |
Sorry, something went wrong.
|
Ah right, that's a better option! I'm not sure about this:
Do you mean Task.cancel(), or abort with RuntimeError, or coro.close(), or something else? |
Sorry, something went wrong.
|
Sorry, "and cancel" should have been "and context". Will fix in the original. |
Sorry, something went wrong.
|
Do you want to always pop name in 3.13.4? |
Sorry, something went wrong.
|
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? |
Sorry, something went wrong.
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. |
Sorry, something went wrong.
|
Ok I opened up a draft here: #133948 the news fragment is going to be fun, I've left it blank for now |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
This reverts commit 7b0543e.
📚 Documentation preview 📚: https://cpython-previews--133808.org.readthedocs.build/