-
Notifications
You must be signed in to change notification settings - Fork 473
Refactor / all_as_schedule crontab query optimization #879
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 / all_as_schedule crontab query optimization #879
Conversation
for more information, see https://pre-commit.ci
…ion' into refactor/crontab-query-optimization
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #879 +/- ##
==========================================
- Coverage 88.08% 88.07% -0.02%
==========================================
Files 32 32
Lines 965 1006 +41
Branches 100 104 +4
==========================================
+ Hits 850 886 +36
- Misses 98 101 +3
- Partials 17 19 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
for more information, see https://pre-commit.ci
…ion' into refactor/crontab-query-optimization
for more information, see https://pre-commit.ci
This comment was marked as outdated.
This comment was marked as outdated.
…ion' into refactor/crontab-query-optimization # Conflicts: # t/unit/test_schedulers.py
for more information, see https://pre-commit.ci
…ion' into refactor/crontab-query-optimization
|
for more information, see https://pre-commit.ci
…ion' into refactor/crontab-query-optimization
Nice, this seems great 👍. Working as expected with theses changes on my end. |
it would be nice to have part of this addressed, specially the check |
auvipy
left a comment
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.
# Current hour tasks in different timezones should not be excluded
assert task_utc_current.id not in excluded_tasks, (
"UTC current hour task should be included"
)
E AssertionError: UTC current hour task should be included
E assert 13 not in {8, 9, 10, 11, 12, 13, ...}
E + where 13 = <PeriodicTask: utc-current-hour: * 8 * * * (m/h/dM/MY/d) UTC>.id
t/unit/test_schedulers.py:1125: AssertionError
The issue was with the mock aware now not being applied. I have resolved it. |
Azurency
left a comment
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.
This looks fine 👍
This PR enhances the DatabaseScheduler's crontab exclusion logic to properly handle tasks with different timezone settings. The scheduler now correctly converts between timezones when determining which tasks to include in the schedule, making the scheduler more efficient by only loading tasks that might be due soon regardless of their timezone configuration.