Skip to content

Conversation

plammens
Copy link
Contributor

@plammens plammens commented Feb 14, 2019

Added a last keyword argument to the run_repeating method of job queues that takes in a time specifier (in the same formats supported by first). 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 if last is an exact multiple of interval, 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.

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.
@Eldinnie
Copy link
Member

Hi,

Thanks for your contribution. As you can see tests fail on py2. This is because you modified the calculation for _next_t in a way a datetime on py2 will never be accepted.
Can you please fix this?

Pieter

@plammens
Copy link
Contributor Author

plammens commented Feb 28, 2019

Ah! Sorry I missed that. Will fix. By the way, could time objects be stored as datetime.datetime instead of plain timestamps? That would simplify it a bit.

Python 2's datetime module doesn't have the datetime.timestamp()
method. An equivalent calculation has been put in its place.
@plammens
Copy link
Contributor Author

plammens commented Mar 1, 2019

@Eldinnie Should be fixed now. Though Travis keeps failing on the unrelated test_idle.

@Eldinnie Eldinnie changed the base branch from master to V12 June 6, 2019 12:39
@plammens
Copy link
Contributor Author

@Eldinnie Any updates on this? I can make changes if the PR is not up to scratch

@tsnoam tsnoam closed this Aug 23, 2019
@Bibo-Joshi
Copy link
Member

This was closed when the V12 branch was deleted. Would you mind reopening the PR to master, @plammens ? :)

@plammens
Copy link
Contributor Author

@Bibo-Joshi Oh, that makes sense. Reopened as #1497!

Copy link
Member

@plammens Thanks!

@github-actions github-actions bot locked and limited conversation to collaborators Aug 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants