uranusjr commented on a change in pull request #16931:
URL: https://github.com/apache/airflow/pull/16931#discussion_r670821620
##########
File path: airflow/timetables/base.py
##########
@@ -83,6 +83,13 @@ def interval(cls, start: DateTime, end: DateTime) ->
"DagRunInfo":
class Timetable(Protocol):
"""Protocol that all Timetable classes are expected to implement."""
+ @property
+ def interval_description(self) -> [str, None]:
+ """Defines the interval description,
+ eg. for cron '30 21 * * 5', description could be like 'At 09:30 PM,
only on Friday'
+ """
Review comment:
```suggestion
"""Override to describe the interval.
For cron ``'30 21 * * 5'``, description could be like ``'At 09:30
PM, only on Friday'``.
This is used in the web UI.
"""
```
##########
File path: airflow/timetables/interval.py
##########
@@ -79,6 +80,20 @@ class CronDataIntervalTimetable(_DataIntervalTimetable):
def __init__(self, cron: str, timezone: datetime.tzinfo) -> None:
self._schedule = CronSchedule(cron, timezone)
+ @Timetable.interval_description.getter
Review comment:
I think just `@property` would do the same (well technically a bit
different, but arguably better).
##########
File path: airflow/timetables/base.py
##########
@@ -83,6 +83,13 @@ def interval(cls, start: DateTime, end: DateTime) ->
"DagRunInfo":
class Timetable(Protocol):
"""Protocol that all Timetable classes are expected to implement."""
+ @property
+ def interval_description(self) -> [str, None]:
Review comment:
```suggestion
def interval_description(self) -> Optional[str]:
```
Or we can make this `str` and return an empty string when a description is
not available, since an empty description does not make sense in the UI anyway.
Instead of `if dag.timetable.interval_description is not None` we can just do
`if not dag.timetable.interval_description`.
##########
File path: airflow/models/dag.py
##########
@@ -2404,6 +2404,20 @@ def __repr__(self):
def timezone(self):
return settings.TIMEZONE
+ @property
+ def timetable(self) -> Timetable:
+ interval = self.schedule_interval
+ if interval is None:
+ return NullTimetable()
+ if interval == "@once":
+ return OnceTimetable()
+ if isinstance(interval, (timedelta, relativedelta)):
+ return DeltaDataIntervalTimetable(interval)
+ if isinstance(interval, str):
+ return CronDataIntervalTimetable(interval, self.timezone)
+ type_name = type(interval).__name__
+ raise TypeError(f"{type_name} is not a valid DAG.schedule_interval.")
Review comment:
Instead of duplicating the logic on `DagModel`, we should reuse
`DAG.timetable` instead. But I’m not sure how this should be done.
--
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]
