-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Add last
kwarg to run_repeating
#1345
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
Add last
kwarg to run_repeating
#1345
Conversation
The procedure to convert a given time object to a float containing the "seconds until the next run" was extracted into an auxiliary top-level function named `_to_interval_seconds`. Also, `_put`'s "prep phase" was refactored as follows. A new local identifier named `interval` was created to make its meaning clearer (instead of always reusing `next_t`, which was confusing). The way in which the interval is fetched (and checked for `None`) has also been simplified. Finally, `next_t`'s last assignment is the actual timestamp in which the job is to be run, which is either `previous_t` (if available) or the current time plus the amount stored in `interval`. Finally, `last_t` was renamed to `previous_t` to avoid confusion with upcoming `last` kwarg.
Added a `last` kwarg to `run_repeating` that specifies when the job should last be run. The checking is done before executing the job, but with a small delay buffer, so that if `last` is an exact multiple of `interval`, the last job is still run, even though the actual time is slightly after the theoretical finish time.
Instead of converting to interval first and then adding the current (or previous) time.
Hi, Thanks for your contribution. As you can see tests fail on py2. This is because you modified the calculation for Pieter |
Ah! Sorry I missed that. Will fix. By the way, could time objects be stored as |
Python 2's datetime module doesn't have the datetime.timestamp() method. An equivalent calculation has been put in its place.
@Eldinnie Should be fixed now. Though Travis keeps failing on the unrelated |
@Eldinnie Any updates on this? I can make changes if the PR is not up to scratch |
This was closed when the V12 branch was deleted. Would you mind reopening the PR to master, @plammens ? :) |
@Bibo-Joshi Oh, that makes sense. Reopened as #1497! |
@plammens Thanks! |
Added a
last
keyword argument to therun_repeating
method of job queues that takes in a time specifier (in the same formats supported byfirst
). It specifies the last time the job should be run. Currently, checking if the job should be run or not involves a certain "delay buffer" (so that iflast
is an exact multiple ofinterval
, the internal delays don't mess up the intended behaviour), but it's a temporary solution. Also, I've made some minor refactorings.Tests run correctly from my side. I've added one test for the new feature, but maybe a few more would be in place. The docs haven't been updated yet.
Closes #1333.