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 the incorrect scheduling time for the first run of dag #21011
Conversation
airflow/timetables/interval.py
Outdated
return max(new_start, earliest) | ||
return max(new_start, self._get_next(earliest)) |
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 needs a test to ensure if earliest
lies exactly on the schedule, the first run is not one interval late. My intuition is this should use self._align
instead, but a test would be required to verify that.
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.
should use anlign. 😢
738f929
to
00c3b4c
Compare
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.
Logic looks good to me, but the tests need some improvements. Also need to fix linter errors.
d21a721
to
af772ad
Compare
|
||
HOURLY_CRON_TIMETABLE = CronDataIntervalTimetable("@hourly", TIMEZONE) | ||
HOURLY_TIMEDELTA_TIMETABLE = DeltaDataIntervalTimetable(datetime.timedelta(hours=1)) | ||
HOURLY_RELATIVEDELTA_TIMETABLE = DeltaDataIntervalTimetable(dateutil.relativedelta.relativedelta(hours=1)) | ||
|
||
CRON_TIMETABLE = CronDataIntervalTimetable("30 16 * * *", TIMEZONE) | ||
CRON_INTERVAL = datetime.timedelta(minutes=30, hours=16) |
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 CRON_INTERVAL
name is confusing; the “interval” of the timetable is actually 24 hours (because jobs run everyday at 16:30). Let’s change it (although I’m not sure what name is appropriate).
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.
Yep, you are right. How about using INTERVAL_FROM_ZERO?:smiley:
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.
Maybe DELTA_FROM_MIDNIGHT
(zero sounds wrong)?
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.
Oh, Great! Use it
af772ad
to
0affef0
Compare
When Catchup_by_default is set to false and start_date in the DAG is the previous day, the first schedule time for this DAG may be incorrect
0affef0
to
090b873
Compare
Happy New Year! |
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease. |
Just an error that we already fixed. Merging |
When Catchup_by_default is set to false and start_date in the DAG is the previous day, the first schedule time for this DAG may be incorrect Co-authored-by: wanlce <who@foxmail.com> (cherry picked from commit 0bcca55)
When Catchup_by_default is set to false and start_date in the DAG is the previous day, the first schedule time for this DAG may be incorrect Co-authored-by: wanlce <who@foxmail.com> (cherry picked from commit 0bcca55)
When Catchup_by_default is set to false and start_date in the DAG is the previous day, the first schedule time for this DAG may be incorrect Co-authored-by: wanlce <who@foxmail.com> (cherry picked from commit 0bcca55)
When catchup_by_default is set to false and the start_date in the dag is the previous day, the first dispatch time of this dag is incorrect
dag setting
error
Correct
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.