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 tried this before, I had DAG.schedule_interval == None and
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 doing this runs afoul of some tests and will be
more work, and 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]