Skip to content

Conversation

@borisalekseev
Copy link

Cron is insensitive to seconds. But cron_offset accepts datetime.timedelta, which has seconds in it. This PR adds sensitivity to seconds and milliseconds from cron_offset.

additional_seconds = 0
if task.cron_offset and isinstance(task.cron_offset, timedelta):
now += task.cron_offset
additional_seconds += task.cron_offset.seconds + (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you use seconds, which might produce unexpected results.

Suggested change
additional_seconds += task.cron_offset.seconds + (
additional_seconds += task.cron_offset.total_seconds() + (

In this example if you specify minutes and hours it will also correctly work. Or was it implemented like so on purpose?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was an even worse mistake here, as you can see. The idea was that additional_seconds is the remainder of the seconds from the entire cron_offset. Now fixed it.

@borisalekseev
Copy link
Author

this will not work correctly with an update-interval greater than a second

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

Successfully merging this pull request may close these issues.

2 participants