Issue31556
Created on 2017-09-22 20:47 by hellysmile, last changed 2017-10-05 16:59 by yselivanov. This issue is now closed.
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 3703 | merged | hellysmile, 2017-09-22 20:51 | |
| Messages (8) | |||
|---|---|---|---|
| msg302770 - (view) | Author: Viktor Kovtun (hellysmile) * | Date: 2017-09-22 20:47 | |
There is no need to create extra future and use loop.call_later to cancel base future from asyncio.wait_for when timeout==0. If loop is heavy loaded it can be cancelled simnifically later then 0 seconds later. |
|||
| msg302771 - (view) | Author: Yury Selivanov (yselivanov) * | Date: 2017-09-22 20:55 | |
Do you have a use case where this optimization is important? |
|||
| msg302772 - (view) | Author: Viktor Kovtun (hellysmile) * | Date: 2017-09-22 21:00 | |
If coroutine function has some blocking calls before first await/yield from statement maybe makes sense do no let them be executed, if timeout equals 0 |
|||
| msg302773 - (view) | Author: Yury Selivanov (yselivanov) * | Date: 2017-09-22 21:08 | |
I think this is a backwards incompatible change and thus will be rejected. Currently there's a guarantee that "wait_for" can throw a TimeoutError *only* when when you await it.
fut = wait_for(something, 0)
# some important code
try:
await fut
except TimeoutError:
# do something
With your PR merged, the above asyncio code would be broken, because asyncio users can guard with try..except only the await expression.
|
|||
| msg302774 - (view) | Author: Viktor Kovtun (hellysmile) * | Date: 2017-09-22 21:18 | |
asyncio.wait_for is coroutine itself, to start executing code, no matter with this PR or not it needs to be awaited/yield from
import asyncio
@asyncio.coroutine
def foo():
print(1)
loop = asyncio.get_event_loop()
fut = asyncio.wait_for(foo(), 0)
print('it is not raised yet')
try:
loop.run_until_complete(fut)
except asyncio.TimeoutError:
print('raised here')
will print
it is not raised yet
raised here
|
|||
| msg302775 - (view) | Author: Yury Selivanov (yselivanov) * | Date: 2017-09-22 21:20 | |
You're right! Let's work on the PR then, I've left a review comment. |
|||
| msg302779 - (view) | Author: Viktor Kovtun (hellysmile) * | Date: 2017-09-22 23:26 | |
Actually provided example without patch will print 1, which is not expected |
|||
| msg303777 - (view) | Author: Yury Selivanov (yselivanov) * | Date: 2017-10-05 16:59 | |
Note, that this will not be backported to 3.6, as it behaves in a slightly incompatible way. I consider this patch as an enhancement that makes 'asyncio.wait_for' semantics easier to reason about and more practical. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2017-10-05 16:59:54 | yselivanov | set | messages: + msg303777 |
| 2017-10-05 16:05:32 | yselivanov | set | status: open -> closed type: behavior resolution: fixed stage: patch review -> resolved |
| 2017-09-22 23:26:44 | hellysmile | set | messages: + msg302779 |
| 2017-09-22 21:20:29 | yselivanov | set | messages: + msg302775 |
| 2017-09-22 21:18:46 | hellysmile | set | messages: + msg302774 |
| 2017-09-22 21:08:22 | yselivanov | set | messages: + msg302773 |
| 2017-09-22 21:00:04 | hellysmile | set | messages: + msg302772 |
| 2017-09-22 20:55:03 | yselivanov | set | messages: + msg302771 |
| 2017-09-22 20:51:42 | hellysmile | set | keywords:
+ patch stage: patch review pull_requests: + pull_request3687 |
| 2017-09-22 20:47:57 | hellysmile | create | |