ashb commented on code in PR #24743:
URL: https://github.com/apache/airflow/pull/24743#discussion_r912877160


##########
airflow/models/dag.py:
##########
@@ -415,17 +419,27 @@ def __init__(
         if 'end_date' in self.default_args:
             self.default_args['end_date'] = 
timezone.convert_to_utc(self.default_args['end_date'])
 
-        # Calculate the DAG's timetable.
-        if timetable is None:
-            self.timetable = create_timetable(schedule_interval, self.timezone)
-            if isinstance(schedule_interval, ArgNotSet):
-                schedule_interval = DEFAULT_SCHEDULE_INTERVAL
-            self.schedule_interval: ScheduleInterval = schedule_interval
-        elif isinstance(schedule_interval, ArgNotSet):
+        # sort out DAG's scheduling behavior
+        scheduling_args = [schedule_interval, timetable, schedule_on]
+        if not at_most_one(*scheduling_args):
+            raise ValueError(f"At most one allowed for args {scheduling_args}")
+
+        self.timetable: Timetable
+        self.schedule_interval: ScheduleInterval
+        self.schedule_on = schedule_on
+        if schedule_on:
+            if not isinstance(schedule_on, list):

Review Comment:
   As TP said, limiting this to a list is overly restrictive -- a sequence is 
the only limit we need.



##########
airflow/models/dag.py:
##########
@@ -415,17 +419,27 @@ def __init__(
         if 'end_date' in self.default_args:
             self.default_args['end_date'] = 
timezone.convert_to_utc(self.default_args['end_date'])
 
-        # Calculate the DAG's timetable.
-        if timetable is None:
-            self.timetable = create_timetable(schedule_interval, self.timezone)
-            if isinstance(schedule_interval, ArgNotSet):
-                schedule_interval = DEFAULT_SCHEDULE_INTERVAL
-            self.schedule_interval: ScheduleInterval = schedule_interval
-        elif isinstance(schedule_interval, ArgNotSet):
+        # sort out DAG's scheduling behavior
+        scheduling_args = [schedule_interval, timetable, schedule_on]
+        if not at_most_one(*scheduling_args):
+            raise ValueError(f"At most one allowed for args {scheduling_args}")
+
+        self.timetable: Timetable
+        self.schedule_interval: ScheduleInterval
+        self.schedule_on = schedule_on
+        if schedule_on:
+            if not isinstance(schedule_on, list):

Review Comment:
   As TP said (somewhere?), limiting this to a list is overly restrictive -- a 
sequence is the only limit we need.



-- 
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]

Reply via email to