sharetheknowledge commented on code in PR #69869:
URL: https://github.com/apache/airflow/pull/69869#discussion_r3740272399
##########
task-sdk/src/airflow/sdk/definitions/dag.py:
##########
@@ -147,7 +147,7 @@ def _create_timetable(interval: ScheduleInterval, timezone:
Timezone | FixedTime
if interval == "@continuous":
return ContinuousTimetable()
if isinstance(interval, timedelta | relativedelta):
- if airflow_conf.getboolean("scheduler", "create_cron_data_intervals"):
+ if airflow_conf.getboolean("scheduler", "create_delta_data_intervals"):
Review Comment:
Added all four. `69869.significant.rst` names who is affected - 2.x migrants
who
set `create_cron_data_intervals = True` on the upgrade guide's advice, plus
DAGs
rebuilt through `conversion_v1_to_v2` - and gives the remedy, including your
point that setting `create_delta_data_intervals = True` is a no-op until this
lands, so it can go in the config before upgrading.
The other three: a bullet in `upgrading_to_airflow3.rst` directly after the
cron
one, `create_delta_data_intervals` added to the "Switching between trigger
and
data interval timetables" list in `timetable.rst`, and the skip-one-period
paragraph on the `config.yml` delta entry. Also bumped "in two ways" to
"three
ways" in that `timetable.rst` list, since it's now three bullets.
##########
task-sdk/tests/task_sdk/definitions/test_dag.py:
##########
@@ -447,6 +447,46 @@ def test_continuous_schedule_linmits_max_active_runs(self):
with pytest.raises(ValueError, match="ContinuousTimetable requires
max_active_runs <= 1"):
dag = DAG("continuous", start_date=DEFAULT_DATE,
schedule="@continuous", max_active_runs=25)
+ def
test_timedelta_schedule_respects_create_delta_data_intervals_config(self):
+ """Regression guard: create_delta_data_intervals must control
DeltaTriggerTimetable vs
+ DeltaDataIntervalTimetable for timedelta/relativedelta schedules,
independently of
+ create_cron_data_intervals (which governs only cron-string schedules).
+ """
+ from airflow.sdk.definitions.timetables.interval import
DeltaDataIntervalTimetable
+ from airflow.sdk.definitions.timetables.trigger import
DeltaTriggerTimetable
+
+ from tests_common.test_utils.config import conf_vars
Review Comment:
Done - moved up to module level with the other `tests_common` import.
##########
task-sdk/tests/task_sdk/definitions/test_dag.py:
##########
@@ -447,6 +447,46 @@ def test_continuous_schedule_linmits_max_active_runs(self):
with pytest.raises(ValueError, match="ContinuousTimetable requires
max_active_runs <= 1"):
dag = DAG("continuous", start_date=DEFAULT_DATE,
schedule="@continuous", max_active_runs=25)
+ def
test_timedelta_schedule_respects_create_delta_data_intervals_config(self):
Review Comment:
Parametrized over `(delta, cron, expected)`, with a second stacked
parametrize
for the schedule type so the three config combinations each run against
`timedelta(days=1)` and `relativedelta(days=1)`. Six independent cases now,
so a
failure in one no longer hides the others.
The `relativedelta` rows earn their place: against the pre-fix code four of
the
six fail (both `delta-true` rows and both `cron-true` rows), the
`relativedelta`
ones included.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
