-
-
Notifications
You must be signed in to change notification settings - Fork 32k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gh-128307: support eager_start kwarg in create_eager_task_factory, and pass kwargs from asyncio.create_task and TaskGroup.create_task #128306
Conversation
c809133
to
2f101b3
Compare
Misc/NEWS.d/next/Library/2024-12-28-11-01-36.gh-issue-128307.BRCYTA.rst
Outdated
Show resolved
Hide resolved
|
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
Outdated
Show resolved
Hide resolved
Misc/NEWS.d/next/Library/2024-12-28-11-01-36.gh-issue-128307.BRCYTA.rst
Outdated
Show resolved
Hide resolved
Misc/NEWS.d/next/Library/2024-12-28-11-01-36.gh-issue-128307.BRCYTA.rst
Outdated
Show resolved
Hide resolved
0699ae2
to
7bce401
Compare
So,
The exception would be unfortunate, no? Since the task factory is global state, there's no way to use the type system to help either. |
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 |
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 now must handle this. |
Ah I see, yes then an exception in that case is the current behaviour |
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 |
I know why the tests fail, will fix in a moment. |
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! |
W00t! Goodnight everyone. Welcome to beta 1. |
|
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 on https://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 adds eager_start
and **kwargs
. In fact this should be done to all the create_task definitions found in typeshed's asyncio definitions.
Uh oh!
There was an error while loading. Please reload this page.