-
Notifications
You must be signed in to change notification settings - Fork 476
refactor: tzaware crontab is due method #899
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
refactor: tzaware crontab is due method #899
Conversation
# Conflicts: # django_celery_beat/schedulers.py
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #899 +/- ##
==========================================
- Coverage 88.24% 88.17% -0.08%
==========================================
Files 32 32
Lines 1012 1006 -6
Branches 105 104 -1
==========================================
- Hits 893 887 -6
Misses 101 101
Partials 18 18 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request refactors the TzAwareCrontab.is_due() method to leverage the superclass implementation and better respect the beat_cron_starting_deadline configuration, while adding associated unit tests for various timing scenarios.
- Refactored TzAwareCrontab.is_due() to call super().is_due()
- Added several unit tests covering edge cases related to the beat_cron_starting_deadline
- Adjusted docstring formatting in tests for clarity
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| t/unit/test_schedulers.py | Added new unit tests for TzAwareCrontab behavior with deadlines |
| django_celery_beat/tzcrontab.py | Simplified is_due() to delegate functionality to the superclass |
Comments suppressed due to low confidence (1)
django_celery_beat/tzcrontab.py:38
- [nitpick] Consider updating the is_due() docstring to indicate that the method now directly defers to the superclass implementation (which handles the beat_cron_starting_deadline logic) to avoid potential confusion.
"""Calculate when the next run will take place."""
| @patch("django_celery_beat.schedulers.aware_now") | ||
| def test_server_timezone_handling_with_zoneinfo(self, mock_aware_now): | ||
| """Test handling when server timezone is already a ZoneInfo instance.""" | ||
| """Test handling when server timezone |
Copilot
AI
May 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider reformatting the multi-line docstring in the test_server_timezone_handling_with_zoneinfo test for consistency and clarity.
Description
This pull request refactors TzAwareCrontab class's is_due() method to adhere to its super class implementation (taking into account the beat_cron_starting_deadline configuration)
Changes
Related Issues
#809