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

FeatReq: Support timedelta for delays? #522

Closed
dragonpaw opened this issue Jan 5, 2023 · 4 comments
Closed

FeatReq: Support timedelta for delays? #522

dragonpaw opened this issue Jan 5, 2023 · 4 comments
Labels

Comments

@dragonpaw
Copy link

dragonpaw commented Jan 5, 2023

In GoLang they have a Duration class that is actually pretty nifty. Instead of passing a int of seconds to sleep, you pass a Duration. Python's similar timedelta class doesn't get much love, but I enjoy using it in place of ints for time within my own code a lot. I notice in Dramatiq, the code wants timeouts and delays passed as millis, which makes sense, but isn't intuitive perhaps from the hinting. So I was wondering if Dramatiq could also accept timedeltas and use those, as there's no ambiguity in them. (While still treating ints as millis for backwards compatibility.)

It'd look something like this:

actor.send_with_options(args=(url), delay=timedelta(seconds=2))

And within the code it's just:

def send_with_options(
    self, *,
    args: tuple = (),
    kwargs: Optional[Dict[str, Any]] = None,
    delay: Optional[Union[datetime.timedelta,int]] = None,
    **options,
) -> Message[R]:
    if isinstance(delay, timedelta):
        delay = delay.total_seconds() * 1000

Funding

  • You can sponsor this specific effort via a Polar.sh pledge below
  • We receive the pledge once the issue is completed & verified
Fund with Polar
@odigity
Copy link

odigity commented Feb 18, 2023

I like it.

Since you've already written the code, how about opening a PR for it? (and maybe a test or two) Looks like the maintainer already has his hands full...

@Bogdanp
Copy link
Owner

Bogdanp commented Mar 25, 2023

I think this is a good idea and a PR would be appreciated.

@arseniiarsenii
Copy link

Hi, @h3nnn4n, I noticed, that the changes are not consistent with other parts of the project which also accept delays in milliseconds. timedelta should also be acceptable as:

  • actor's min_backoff parameter
  • actor's max_backoff parameter
  • dramatiq.errors.Retry exception's 'delay' argument

These are some places that come to my mind, there are probably more of them.

Otherwise, thanks for the PR!

@Bogdanp
Copy link
Owner

Bogdanp commented Apr 28, 2024

I think the request here has been covered so I'm going to close this for now. If someone wants to extend the other places where we could use timedeltas, feel free to open a PR.

@Bogdanp Bogdanp closed this as completed Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants