Skip to content
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-128289: Add eager_tasks parameter to asyncio.run() and asyncio.Runner #128293

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

asvetlov
Copy link
Contributor

@asvetlov asvetlov commented Dec 27, 2024

@asvetlov
Copy link
Contributor Author

@gvanrossum does the PR fit your vision?
Does eager_task argument sound good or there is better naming?

@asvetlov
Copy link
Contributor Author

asvetlov commented Dec 27, 2024

A failed hypothesis test for glob is definitely not related to the PR, it is broken in the main branch already.

@graingert
Copy link
Contributor

This will also need some way to set this in IsolatedAsyncioTestCase

@asvetlov
Copy link
Contributor Author

This will also need some way to set this in IsolatedAsyncioTestCase

Good idea. A classvar attribute with special name maybe, something like EAGER_TASKS = True.

I would update async test case when this PR will land.

@graingert
Copy link
Contributor

graingert commented Dec 27, 2024

Possibly we could stick to just using loop_factory for this and asyncio.Task can issue the deprecation warning if eager isn't set to True in the constructor. (We could use eager = True, False or None to decide the warning)

@asvetlov
Copy link
Contributor Author

Possibly we could stick to just using loop_factory for this and asyncio.Task can issue the deprecation warning if eager isn't set to True in the constructor. (We could use eager = True, False or None to decide the warning)

Sorry, I don't follow how loop_factory is related to lazy/eager task creation.

Could you please elaborate?

@graingert
Copy link
Contributor

graingert commented Dec 27, 2024

Currently for 3.13 to enable eager tasks the approach is to do:

def loop_factory():
    loop = asyncio.EventLoop()
    loop.set_task_factory(asyncio.eager_task_factory)
    return loop

asyncio.run(amain(), loop_factory=loop_factory)

So this should still work and not trigger a deprecation warning

@asvetlov
Copy link
Contributor Author

I consider loop.set_task_factory() as low-level function.
If a user overrides eager_tasks by set_task_factory() it is totally legal behavior, a user is free to do it on its own.

@graingert
Copy link
Contributor

graingert commented Dec 28, 2024

We could provide an eager loop factory and deprecate calling asyncio.Task(eager_start=None)

Code would look like asyncio.run(amain(), loop_factory=asyncio.eager_loop_factory)

@asvetlov
Copy link
Contributor Author

I've got you finally, I hope.
Yes, it is a viable solution too.
Anyway, we should reach some conclusion in the discuss group first before any of PR will get a chance to be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants