Unify summary/description logic to timetable

[uranusjr]

These values are pulled directly from the timetable, so we can unify the dataset case into it instead of needing extra if-else cases.

[dstandish]

@uranusjr this needs to be 'Dataset' but right now it will be None

[dstandish]

yesterday i tried doing the same thing, but making it dag.timetable.summary --- which would be probably the "right" way -- but then tests started failing so i aborted mission

[uranusjr]

Hmm those two should be the same value (due to the assignment in __init__ so something somewhere is changing the value and we need to find out what that is.

[dstandish]

oh -- maybe i have that wrong! lemme look

[dstandish]

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.

[uranusjr]

Yes the key “problem” per-se is that DAG.schedule_interval at this point does not actually have any functionality beyond determining the label name we show on the UI. A rename here is really needed to reflect that, and this change makes sense if you look pass the attribute name.

[dstandish]

yeah i guess what i am saying though is that while DagModel.schedule_interval does not have meaning and is just a label, DAG.schedule_interval does, and so that's why it feels a little off to store the label Dataset on DAG.schedule_interval. storing on the DagModel attr makes sense but, in a perfect world, we wouldn't also set it on DagModel. anyway, nothing really to worry about.

[dstandish]

while DagModel.schedule_interval does not have meaning and is just a label, DAG.schedule_interval does

er, at least i think it does, doesn't it? like that's where you set @daily or 30 7 * * *

[dstandish]

should have made initial review "request changes" (just to prevent inadvertent premature merge)

[dstandish]

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.