Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.4k
gh-128307: support eager_start kwarg in create_eager_task_factory, and pass kwargs from asyncio.create_task and TaskGroup.create_task#128306
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
c809133
to2f101b3
CompareUh oh!
There was an error while loading.Please reload this page.
Misc/NEWS.d/next/Library/2024-12-28-11-01-36.gh-issue-128307.BRCYTA.rst OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
|
yup, it's in I just forgot to add it to the news |
Misc/NEWS.d/next/Library/2024-12-28-11-01-36.gh-issue-128307.BRCYTA.rst OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Misc/NEWS.d/next/Library/2024-12-28-11-01-36.gh-issue-128307.BRCYTA.rst OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Misc/NEWS.d/next/Library/2024-12-28-11-01-36.gh-issue-128307.BRCYTA.rst OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
0699ae2
to7bce401
CompareUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
graingert commentedMay 4, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
How would the loop even know what type task factory was installed so as to raise? |
Assuming the segfault will be fixed, let's just work through adding eager_start to all create_task methods. (We can test with the crux of that fix patched in.) @graingert Do you need help writing any code? One thing I'd like to ensure is that at least It might be possible to come up with a protocol whereby a task factory communicates which |
graingert commentedMay 5, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I strongly disagree with raising an exception in the currently supported loop.create_task(eager_start=True) case. |
So what would you have happen if the task factory doesn't support eager starts? This API is a breaking change if lazy task factories nowmust handle this. |
Ah I see, yes then an exception in that case is the current behaviour |
Uh oh!
There was an error while loading.Please reload this page.
python-cla-botbot commentedMay 5, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
gvanrossum-ms commentedMay 5, 2025
But by looking at more of the code I just learned that the eagerness is purely implemented by passing There's one bit of behavior that's changed -- not sure if we care (the docs do not mention it): if you explicitly use |
gvanrossum-ms commentedMay 5, 2025
I know why the tests fail, will fix in a moment. |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@graingert Are you okay if I merge this once the tests pass?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Just giving everything a final look over
LGTM! Thanks! |
08d7687
intopython:mainUh oh!
There was an error while loading.Please reload this page.
W00t! Goodnight everyone. Welcome to beta 1. |
bedevere-bot commentedMay 5, 2025
|
The |
It's already fixed, here's the issue:#133419 |
@@ -386,19 +386,13 @@ def __wakeup(self, future): | |||
Task = _CTask = _asyncio.Task | |||
def create_task(coro, *, name=None, context=None): | |||
def create_task(coro, **kwargs): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
From the Python user perspective, I don't like this change.
It obscures the allowed kwargs and makes it harder for IDEs to provide suggestions.
I suppose we'd be relying onhttps://github.com/python/typeshed/blob/main/stdlib/asyncio/tasks.pyi from now on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I agree it would be nice if we could avoid the**kwargs
, but it make the implementation more complex at every level of indirection (asyncio.create_task -> loop.create_task -> Task, and also TaskGroup.create_task -> loop.create_task -> Task).
IDEs should be using typeshed anyway. (And we should update tasks.pyi to have a case for 3.14 that addseager_start
and**kwargs
. In fact this should be done to all the create_task definitions found in typeshed's asyncio definitions.
…ory and various create_task functions (python#128306)Some create_task() functions were changed from `name=None, context=None` to `**kwargs`.Co-authored-by: Guido van Rossum <gvanrossum@gmail.com>
Uh oh!
There was an error while loading.Please reload this page.