dstandish commented on code in PR #25045:
URL: https://github.com/apache/airflow/pull/25045#discussion_r921397899
##########
airflow/models/dag.py:
##########
@@ -2498,12 +2498,8 @@ def bulk_write_to_db(cls, dags: Collection["DAG"],
session=NEW_SESSION):
orm_dag.max_active_tasks = dag.max_active_tasks
orm_dag.max_active_runs = dag.max_active_runs
orm_dag.has_task_concurrency_limits = any(t.max_active_tis_per_dag
is not None for t in dag.tasks)
- if dag.schedule_on:
- orm_dag.schedule_interval = 'Dataset'
- orm_dag.timetable_description = 'Triggered by datasets.'
- else:
- orm_dag.schedule_interval = dag.schedule_interval
- orm_dag.timetable_description = dag.timetable.description
+ orm_dag.schedule_interval = dag.schedule_interval
Review Comment:
ok so ... here's what's going on... i didn't see that you set
DAG.schedule_interval = 'Dataset' in `DAG.__init__`
so i assumed it would be None here.
the way i did it, I had `DAG.schedule_interval == None` but
`DagModel.schudule_interval == 'Dataset'`, with the thought that
`DAG.schedule_interval` _should be_ `None`, while `DagModel.schedule_interval`
would be better named `DagModel.schedule_label` or something.
the idea is that there are two different things going on here. one is, the
"label" presented under "schedule" column in dags page. other is, the
`schedule_interval` param for DAG object -- which for datasets is None -- cus
there is _no_ schedule interval for this kind of dag. it may best to make
clear this distinction and leave DAG.schedule_interval `None` while setting
DagModel.schedule_interval to `Dataset` (and perhaps renaming
DagModel.schedule_interval to DagModel.schedule_summary or
DagModel.schedule_label). But _that_ will require a lot more work, we need not
let perfect stand in the way of good, so i'll approve.
--
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]
