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

Fix: timezone info occasionally removed from cron job execution time #383

Merged
merged 2 commits into from
Apr 1, 2024

Conversation

iamlikeme
Copy link
Contributor

@iamlikeme iamlikeme commented Dec 18, 2022

This PR fixes a bug where timezone information was occasionally removed from execution time of cron jobs. The bug affected cron jobs which used the month-rule and when month was incremented due to that rule.

Since #354 all cron calculations are done on timezone aware datetimes. However, the tests for next_cron function were not updated to include timezone in all test cases, which is how the bug slipped through. The tests are updated now to always use timezone.

Fixes: #380

@codecov
Copy link

codecov bot commented Dec 18, 2022

Codecov Report

Merging #383 (4a0e151) into main (ab2dda2) will not change coverage.
The diff coverage is 100.00%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #383   +/-   ##
=======================================
  Coverage   98.40%   98.40%           
=======================================
  Files          11       11           
  Lines        1063     1063           
  Branches      200      200           
=======================================
  Hits         1046     1046           
  Misses          8        8           
  Partials        9        9           
Files Coverage Δ
arq/cron.py 100.00% <100.00%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ab2dda2...4a0e151. Read the comment docs.

@JonasKs
Copy link
Collaborator

JonasKs commented Dec 18, 2022

Tests has been fixed in #382, so don't worry about the pipelines failing for some Python versions.

@iamlikeme iamlikeme force-pushed the fix-next-cron branch 2 times, most recently from 55d9d5b to b50c1dc Compare November 30, 2023 20:41
@iamlikeme
Copy link
Contributor Author

Ready for merging 🙏

@samuelcolvin samuelcolvin merged commit 94cd878 into python-arq:main Apr 1, 2024
10 checks passed
@samuelcolvin
Copy link
Member

thanks so much, sorry for the long delay.

@samuelcolvin
Copy link
Member

v0.26.0b1 is released, please try it, I'll release v0.26 at the end of the week, see #441.

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.

Datetime in next_cron func doesn't have timezone information
4 participants