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

Make timeout duration unit self-documenting #80

Open
l0b0 opened this issue Jan 27, 2021 · 2 comments
Open

Make timeout duration unit self-documenting #80

l0b0 opened this issue Jan 27, 2021 · 2 comments

Comments

@l0b0
Copy link

l0b0 commented Jan 27, 2021

In a statement like @mark.timeout(50) there is nothing to say the duration is defined in seconds. This has a few disadvantages:

  • Many other timing mechanisms use milliseconds or minutes as their units, so we can't simply assume that it's seconds. A new or returning user has to read the documentation (or code) to ascertain this, wasting time and producing cognitive overhead.
  • Extreme timeouts like 1200 or 0.00005 have more cognitive overhead, since converting to the "natural" unit of minutes/microseconds etc isn't always trivial.

It would be nice if it instead were self-documenting. Some alternatives:

  • @mark.timeout(seconds=50) would make it pretty obvious, and would make it easy to support several units.
  • @mark.timeout(timedelta(minutes=5)) would make it more explicit, while supporting the widest possible range of units at no extra cost.
  • @mark.timeout(seconds=timedelta(minutes=5).total_seconds()) is a clunky workaround.
@flub
Copy link
Member

flub commented Oct 8, 2023

I think a PR allowing to use timedelta in there would be acceptable.

@s0undt3ch
Copy link
Contributor

Here's an idea...

marker = item.get_closest_marker(name="timeout")
if marker.args:
    # Old marker
    if len(marker.args) > 1:
        raise RuntimeError("The `timeout` marker only accepts one argument.")
    if marker.kwargs:
        raise RuntimeError("The `timeout` marker does not accept keyword arguments when arguments are passed.")
    timeout = timedelta(seconds=marker.args[0])
elif marker.kwargs:
    timeout = timedelta(**marker.kwargs)
else:
    raise RuntimeError("The `timeout` marker needs either argument or keyword arguments passed.")

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

No branches or pull requests

3 participants