Add support for multiple cron expressions in schedule_interval #24733

BasPH · 2022-06-29T13:06:41Z

This PR extends the behavior of CronDataIntervalTimetable to support a collection (list/set/tuple) of strings to the schedule_interval. For example:

with DAG(dag_id="foobar", schedule_interval=["@daily", "0 3 * * MON,TUE"]):
    ...

^ 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 a newsfragement file, named {pr_number}.significant.rst, in newsfragments.

ashb

My only question is is it worth exposing this via schedule_interval or should we make people use a timetable directly for this?

Do we also need a new timetable class, or could the existing CornDataIntervalTimetable be extended to take multiple patterns, and then all we change is what gets passed when upgrading schedule_interval.?

You also haven't provided a description field (which is IIRC what is shown in the UI) for the new timetable class.

ashb · 2022-06-29T13:21:49Z

docs/apache-airflow/concepts/dags.rst

+------------------------------------------+----------------------------------------------------------------------------------------------------------------------+--------------------------------------+
+| Cron preset (``str``)                    | Convenience cron expression for readability                                                                          + ``"@daily"``                         |
+------------------------------------------+----------------------------------------------------------------------------------------------------------------------+--------------------------------------+
+| List of cron expressions/presets         | To run at intervals that cannot be expressed by a single cron expression.                                            + ``["0 3 * * *", "0 0 * * MON,TUE"]`` |


This should say if the mode is "and" or "or" (as implemented it's "or")

I can be both "and" and "or". For example, when given ["0 3 * * MON,TUE", "@daily"], each expression is looked up in the cron presets and converted if found. Shall I convert to "and/or"?

Updated the example to demonstrate ["@daily", "0 3 * * MON,TUE"] is a valid schedule_interval.

docs/apache-airflow/concepts/dags.rst

BasPH · 2022-06-29T14:35:32Z

My only question is is it worth exposing this via schedule_interval or should we make people use a timetable directly for this?

I believe it's more convenient exposing it via schedule_interval since (1) that's currently how other timetables are exposed and (2) timetables to me feel like an implementation detail with lots of intricate details. I believe multiple crons is a common ask and therefore not worth requiring the user to initialize a timetable.

Do we also need a new timetable class, or could the existing CornDataIntervalTimetable be extended to take multiple patterns, and then all we change is what gets passed when upgrading schedule_interval.?

We could extend the existing implementation to take a list of cron expressions (and convert a single string to a list of strings internally to align the business logic).

You also haven't provided a description field (which is IIRC what is shown in the UI) for the new timetable class.

Added.

uranusjr · 2022-06-30T05:35:37Z

Exposing this in schedule_interval makes sense to me; this is probably the only semantic that makes sense anyway.

I would propose we accept any Collection[str] input, instead of just list. Passing in a set makes as much sense here as list, maybe even more?

BasPH · 2022-06-30T07:48:52Z

Merged the MultiCronDataIntervalTimetable into the CronDataIntervalTimetable, which now accepts both a single string and a list of strings.

Converted the PR to draft for the moment because I had to change the logic on _should_fix_dst and plan to add tests to ensure correct functioning.

@uranusjr Agreed, will add.

BasPH · 2022-07-01T11:10:08Z

@uranusjr Added support for sets (& tuples). Didn't set the type to Collection though because that also includes dicts, which is not supported. Since there's no special type for Collections expect dicts (as far as I know?) the type is now Union[List[str], Set[str], Tuple[str, ...]], does that work for you?

…rval

BasPH · 2022-07-20T20:00:26Z

@uranusjr @ashb any more thoughts on this?

uranusjr · 2022-07-22T09:09:16Z

airflow/models/dag.py

@@ -105,7 +105,8 @@


 DagStateChangeCallback = Callable[[Context], None]
-ScheduleInterval = Union[None, str, timedelta, relativedelta]
+MultiCron = Union[List[str], Set[str], Tuple[str, ...]]


This additional type alias doesn’t seem to be necessary?

(Also this can probably be simply Collection[str])

uranusjr · 2022-07-22T09:10:02Z

airflow/models/dag.py

+    if isinstance(interval, str) or (
+        isinstance(interval, (list, set, tuple)) and all(isinstance(element, str) for element in interval)
+    ):


Suggested change

if isinstance(interval, str) or (

isinstance(interval, (list, set, tuple)) and all(isinstance(element, str) for element in interval)

):

if isinstance(interval, str) or (

isinstance(interval, Collection) and all(isinstance(element, str) for element in interval)

):

uranusjr · 2022-07-22T09:11:04Z

airflow/models/dag.py

+            orm_dag.schedule_interval = (
+                list(dag.schedule_interval)
+                if isinstance(dag.schedule_interval, set)
+                else dag.schedule_interval
+            )


Why is it necessary to special-case set?

uranusjr · 2022-07-22T09:11:56Z

airflow/timetables/interval.py

+    def __init__(
+        self, crons: Union[str, List[str], Set[str], Tuple[str, ...]], timezone: Union[str, Timezone]
+    ) -> None:


Instead of normalizing here, I wonder if it’s easier to make this only accept List[str] and normalize in DAG.__init__ instead.

uranusjr · 2022-07-22T09:13:07Z

docs/apache-airflow/concepts/dags.rst

@@ -197,6 +210,9 @@ schedule interval put in place, the logical date is going to indicate the time
 at which it marks the start of the data interval, where the DAG run's start
 date would then be the logical date + scheduled interval.

+.. tip::
+    For more information on ``logical date``, see :ref:`data-interval` and :ref:`faq:what-does-execution-date-mean`.


Suggested change

For more information on ``logical date``, see :ref:`data-interval` and :ref:`faq:what-does-execution-date-mean`.

For more information on *logical date*, see :ref:`data-interval` and :ref:`faq:what-does-execution-date-mean`.

nit (this word is not code so let’s not make it look like code)

github-actions · 2022-09-06T00:13:50Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

Inetov · 2023-07-11T10:29:18Z

Hi @BasPH !
what does it take to finish this? very useful feature!

moreaupascal56 · 2023-07-12T12:17:15Z

Hi all,
+1 for @Inetov, I can try to help on this as well. I am using this timetable for now (https://stackoverflow.com/a/72492370) but a native option would be great 👍

jprumsey · 2023-07-19T14:31:17Z

+1, native option here would be great

uranusjr · 2023-07-24T03:27:31Z

Please refrain from posting messages such as “+1” since it does not add to the conversation nor move forward the feature. If you are interested in seeing the functionality, please start a pull request yourself.

BasPH requested review from uranusjr, kaxil, potiuk, ashb and XD-DENG as code owners June 29, 2022 13:06

boring-cyborg bot added area:serialization kind:documentation labels Jun 29, 2022

ashb reviewed Jun 29, 2022

View reviewed changes

BasPH marked this pull request as draft June 30, 2022 07:45

BasPH force-pushed the multicron branch from 69d439f to bd76fe7 Compare June 30, 2022 14:46

BasPH marked this pull request as ready for review June 30, 2022 21:34

BasPH force-pushed the multicron branch from 5c848c1 to e30e677 Compare June 30, 2022 21:34

eladkal mentioned this pull request Jul 7, 2022

Allow user to supply croniter object, multiple cron strings or generator object to DAG #8835

Closed

BasPH added 11 commits July 20, 2022 21:51

Add multicron timetable

bc98eb8

Support multiple cron expressions in DAG serialization

2c5afd8

Update docs to show multicron

a876ca3

Remove accidental naive conversion, copy paste error

88d4635

Set timetable description

7c4d791

Merge MultiCronDataIntervalTimetable into CronDataIntervalTimetable

b7da899

Support sets for supplying multiple cron expressions to schedule_inte…

6e65a73

…rval

Add support for tuples in schedule_interval

716f902

Add back and move tip about logical date to a logical place

8384937

Remove obsolete comment

4209f49

Demonstrate multicron example using mixture of expression & preset

bf1dd91

BasPH force-pushed the multicron branch from 7936951 to bf1dd91 Compare July 20, 2022 19:57

uranusjr reviewed Jul 22, 2022

View reviewed changes

github-actions bot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Sep 6, 2022

github-actions bot closed this Sep 11, 2022

Inetov mentioned this pull request Nov 1, 2023

Added support for multiple cron expressions #35337

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for multiple cron expressions in schedule_interval #24733

Add support for multiple cron expressions in schedule_interval #24733

BasPH commented Jun 29, 2022 •

edited

Loading

ashb left a comment

ashb Jun 29, 2022

BasPH Jun 29, 2022

BasPH Jul 1, 2022

BasPH commented Jun 29, 2022

uranusjr commented Jun 30, 2022

BasPH commented Jun 30, 2022

BasPH commented Jul 1, 2022

BasPH commented Jul 20, 2022

uranusjr Jul 22, 2022

uranusjr Jul 22, 2022

uranusjr Jul 22, 2022

uranusjr Jul 22, 2022

uranusjr Jul 22, 2022

github-actions bot commented Sep 6, 2022

Inetov commented Jul 11, 2023

moreaupascal56 commented Jul 12, 2023

jprumsey commented Jul 19, 2023

uranusjr commented Jul 24, 2023

	For more information on ``logical date``, see :ref:`data-interval` and :ref:`faq:what-does-execution-date-mean`.
	For more information on logical date, see :ref:`data-interval` and :ref:`faq:what-does-execution-date-mean`.

Add support for multiple cron expressions in schedule_interval #24733

Add support for multiple cron expressions in schedule_interval #24733

Conversation

BasPH commented Jun 29, 2022 • edited Loading

ashb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BasPH commented Jun 29, 2022

uranusjr commented Jun 30, 2022

BasPH commented Jun 30, 2022

BasPH commented Jul 1, 2022

BasPH commented Jul 20, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 6, 2022

Inetov commented Jul 11, 2023

moreaupascal56 commented Jul 12, 2023

jprumsey commented Jul 19, 2023

uranusjr commented Jul 24, 2023

BasPH commented Jun 29, 2022 •

edited

Loading