gh-128307: support eager_start kwarg in create_eager_task_factory, and pass kwargs from asyncio.create_task and TaskGroup.create_task

[graingert]

• Issue: add eager_start parameter to loop.create_task #128307

[asvetlov]

TaskGroup.create_task() aslo should accept eager_start argument I guess.

[graingert]

TaskGroup.create_task() aslo should accept eager_start argument I guess.

yup, it's in I just forgot to add it to the news

[graingert]

Shouldn't eager_start=False be added to the default create_task method? The tests show that you can write create_task(coro(), eager_start=<bool>) when an eager task factory is installed. Shouldn't you also be able to do that when one isn't installed?

Well apparently it segfaults when you do that. I'll need to investigate and bisect

[graingert]

a slightly minimized segfault:

import asyncio


async def asyncfn():
    pass

def create_task(loop, coro, **kwargs):
    task = asyncio.Task(coro, loop=loop, **kwargs)
    try:
        return task
    finally:
        # gh-128552: prevent a refcycle of
        # task.exception().__traceback__->BaseEventLoop.create_task->task
        del task

async def main():
    t = create_task(asyncio.get_running_loop(), asyncfn(), eager_start=True, name="example")
    assert t.done()
    await t

asyncio.run(main(), loop_factory=asyncio.EventLoop)

[Tinche]

So,

factory installed	eager_start=None (default)	eager_start=True	eager_start=False
lazy	task started lazily	runtime exception	task started lazily
eager	task started eagerly	task started eagerly	task started lazily

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.

[graingert]

How would the loop even know what type task factory was installed so as to raise?

[gvanrossum]

• The segfault is being worked on, so let's assume it will get fixed in time and continue to work on this as if it's fixed. The Tuesday deadline is very close!
• In response to Tin's nice table, that's exactly what I assume. I am okay with the exception: it represent a reality of the current implementation and I don't want to add work before beta 1. We may be able to fix that in a later feature release.
• Thomas: "How would the loop even know what type task factory was installed so as to raise?" -- I'm not sure what this refers to. Maybe Tin's remark "there's no way to use the type system to help"? But I still don't understand it. Maybe it's not important, so I'll ignore it for now.

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 eager_start=None (even when explicitly given) is not passed to the task factory, to deal with 3rd party task factories that haven't been updated to support this argument.

It might be possible to come up with a protocol whereby a task factory communicates which eager_start values it supports (e.g. a function attribute on the factory), but that seems a future extension (we'll hopefully learn during beta whether this is needed).

[graingert]

I strongly disagree with raising an exception in the currently supported loop.create_task(eager_start=True) case.

[mikeshardmind]

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.

[graingert]

Ah I see, yes then an exception in that case is the current behaviour

[python-cla-bot]

All commit authors signed the Contributor License Agreement.

[gvanrossum-ms]

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 Task(..., eager_start=True) so there is no need to have that exception in the table!

There's one bit of behavior that's changed -- not sure if we care (the docs do not mention it): if you explicitly use create_task(..., context=None) it will pass context=None to the task factory, whereas the original code would not pass it. But is anyone passing context=None explicitly? Ditto for eager_start=None, though we could say that that's the same as False.

[gvanrossum-ms]

I know why the tests fail, will fix in a moment.

[gvanrossum]

@graingert Are you okay if I merge this once the tests pass?

[graingert]

Just giving everything a final look over

[graingert]

LGTM! Thanks!

[gvanrossum]

W00t! Goodnight everyone. Welcome to beta 1.

[bedevere-bot]

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 Debian root 3.x (tier-1) has failed when building commit 08d7687.

What do you need to do:

1. Don't panic.
2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/345/builds/11228) and take a look at the build logs.
4. Check if the failure is related to this commit (08d7687) or if it is a false positive.
5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/345/builds/11228

Failed tests:

• test_decimal
• test_external_inspection

Failed subtests:

• test_async_remote_stack_trace - test.test_external_inspection.TestGetStackTrace.test_async_remote_stack_trace

Summary of the results of the build (if available):

==

Click to see traceback logs

Traceback (most recent call last):
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_external_inspection.py", line 269, in test_async_remote_stack_trace
    self.assertEqual(stack_trace, expected_stack_trace)
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: Lists differ: [[('c[62 chars]y', 10), ('c4', '/tmp/test_python_sp6aoafc/tmp[1293 chars]]]]]] != [[('c[62 chars]y', 11), ('c4', '/tmp/test_python_sp6aoafc/tmp[1308 chars]]]]]]


Traceback (most recent call last):
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_external_inspection.py", line 269, in test_async_remote_stack_trace
    self.assertEqual(stack_trace, expected_stack_trace)
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: Lists differ: [[('c[62 chars]y', 10), ('c4', '/tmp/test_python_9w2frcjy/tmp[1293 chars]]]]]] != [[('c[62 chars]y', 11), ('c4', '/tmp/test_python_9w2frcjy/tmp[1308 chars]]]]]]