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

asyncio.sleep() does not adhere to time.sleep() behavior for negative numbers #83879

Closed
Marco-Sulla mannequin opened this issue Feb 20, 2020 · 7 comments
Closed

asyncio.sleep() does not adhere to time.sleep() behavior for negative numbers #83879

Marco-Sulla mannequin opened this issue Feb 20, 2020 · 7 comments
Labels
3.9 only security fixes topic-asyncio type-bug An unexpected behavior, bug, or error

Comments

@Marco-Sulla
Copy link
Mannequin

Marco-Sulla mannequin commented Feb 20, 2020

BPO 39698
Nosy @asvetlov, @1st1, @Marco-Sulla

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2020-02-20.19:17:33.548>
created_at = <Date 2020-02-20.11:44:23.841>
labels = ['type-bug', '3.9', 'expert-asyncio']
title = 'asyncio.sleep() does not adhere to time.sleep() behavior for negative numbers'
updated_at = <Date 2020-02-26.19:46:37.563>
user = 'https://github.com/Marco-Sulla'

bugs.python.org fields:

activity = <Date 2020-02-26.19:46:37.563>
actor = 'Marco Sulla'
assignee = 'none'
closed = True
closed_date = <Date 2020-02-20.19:17:33.548>
closer = 'yselivanov'
components = ['asyncio']
creation = <Date 2020-02-20.11:44:23.841>
creator = 'Marco Sulla'
dependencies = []
files = []
hgrepos = []
issue_num = 39698
keywords = []
message_count = 7.0
messages = ['362314', '362321', '362345', '362349', '362519', '362673', '362724']
nosy_count = 3.0
nosy_names = ['asvetlov', 'yselivanov', 'Marco Sulla']
pr_nums = []
priority = 'normal'
resolution = 'rejected'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue39698'
versions = ['Python 3.9']

@Marco-Sulla
Copy link
Mannequin Author

Marco-Sulla mannequin commented Feb 20, 2020

Python 3.9.0a3+ (heads/master-dirty:f2ee21d858, Feb 19 2020, 23:19:22) 
[GCC 9.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import time
>>> time.sleep(-1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: sleep length must be non-negative
>>> import asyncio
>>> async def f():
...     await asyncio.sleep(-1)
...     print("no exception")
... 
>>> asyncio.run(f())
no exception

I think that also asyncio.sleep() should raise ValueError if the argument is less than zero.

@Marco-Sulla Marco-Sulla mannequin added 3.9 only security fixes topic-asyncio labels Feb 20, 2020
@asvetlov
Copy link
Contributor

The ship has sailed, this change breaks a lot of existing code without a strong reason.
I recall very many cases in third-party libraries and commercial applications where a negative argument for asyncio.sleep() is processed as asyncio.sleep(0) without failure.

@1st1
Copy link
Member

1st1 commented Feb 20, 2020

The ship has sailed, this change breaks a lot of existing code without a strong reason.

Yes.

It's a common thing to compute asyncio.sleep delay and sometimes it goes negative. The current behavior is part of our API now.

@1st1 1st1 added the invalid label Feb 20, 2020
@1st1 1st1 closed this as completed Feb 20, 2020
@1st1 1st1 added the invalid label Feb 20, 2020
@1st1 1st1 closed this as completed Feb 20, 2020
@Marco-Sulla
Copy link
Mannequin Author

Marco-Sulla mannequin commented Feb 20, 2020

I recall very many cases in third-party libraries and commercial applications

Source?

@Marco-Sulla
Copy link
Mannequin Author

Marco-Sulla mannequin commented Feb 23, 2020

I see that many breaking changes was done in recent releases. I get only the ones for asyncio in Python 3.8:

https://bugs.python.org/issue36921
https://bugs.python.org/issue36373
https://bugs.python.org/issue34790
https://bugs.python.org/issue32528
https://bugs.python.org/issue34687
https://bugs.python.org/issue32314

So I suppose the ship isn't sailed yet.

Passing a negative number to a function that should sleep the task for x seconds is a mistake. And mistakes should never pass silently.

Furthermore, coherence matters. It's really confusing that two functions in two builtin modules that are quite identical have a different behavior.

IMHO, deprecating and then removing support for negative argument in asyncio.sleep() is very much less breaking compared to issues bpo-36921 and bpo-36373 .

@Marco-Sulla Marco-Sulla mannequin added type-bug An unexpected behavior, bug, or error labels Feb 23, 2020
@1st1
Copy link
Member

1st1 commented Feb 25, 2020

Source?

I could not find a good source, sorry. I remember I had a complaint in uvloop to support negative timeouts, but I can't trace it.

That said, I also distinctly remember seeing code (and writing such code myself) that performs computation on timeouts and does not care if the end value goes below 0. It might be a weak data point but it's still a valid one.

IMHO, deprecating and then removing support for negative argument in asyncio.sleep() is very much less breaking compared to issues bpo-36921 and bpo-36373 .

Breaking code/APIs always has a price and we always try to have a very good explanation "why" we want to bother ourselves and users to break backwards compat. This one is not worth it IMHO.

We have more breaking API changes that are more substantial coming in future versions of asyncio. So I try to limit the impact by only breaking what's really necessary.

@Marco-Sulla
Copy link
Mannequin Author

Marco-Sulla mannequin commented Feb 26, 2020

I also distinctly remember seeing code (and writing such code myself) that performs computation on timeouts and does not care if the end value goes below 0.

This is not a good statistics. Frankly we can't measure the impact of the change from these considerations. And furthermore, asyncio.sleep() is used often, testing and mocking apart? I doubt it.

we always try to have a very good explanation "why" we want to bother ourselves and users to break backwards compat.

Coherence and unhide mistakes are *very* strong points.

That said, I'm not so much interested in practice. Do as you wish. The problem is I always considered Python a very elegant programming language, and this behavior is not elegant at all. But, hey, amen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.9 only security fixes topic-asyncio type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

2 participants