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 |
|
All commit authors signed the Contributor License Agreement. |
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.
@graingert Are you okay if I merge this once the tests pass?
|
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.
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.
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.