-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
Refactor asyncio.wait_for() #31847
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
Refactor asyncio.wait_for() #31847
Conversation
|
||
fut = ensure_future(fut, loop=loop) | ||
fut.add_done_callback(cb) | ||
async def inner(): |
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.
@asvetlov I've been having a go at this branch in bf594bd
you need to special case None
in asyncio.wait_for(
async def inner(): | |
if timeout is None: | |
return await fut | |
async def inner(): |
and then special case 0
in timeouts.timeout see https://gist.github.com/graingert/ea2546b23b32be5a4493a9a115db2eff#file-timeout_bug-py-L6
raise exceptions.TimeoutError() from exc | ||
finally: | ||
timeout_handle.cancel() | ||
return await create_task(inner()) |
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 don't think this extra create_task is worth it, the tests it passes are equivalent to:
async def wait_for(fut, delay):
if delay is None:
return await fut
if delay <= 0:
if asyncio.iscoroutine(fut):
fut.close()
raise TimeoutError
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 think it would be better to skip this create_task and delete the tests that fail
No description provided.