Page MenuHomePhabricator

link schedule page
ClosedPublic

Authored by prha on Dec 2 2019, 10:48 PM.

Details

Summary

D1479 created the standalone schedule page. This diff links it from the schedules list view, and formats the attempts table a little bit to take into account pipeline run state.

Test Plan

Viewed schedule page, saw both successful and unsuccessful attempts

Diff Detail

Repository
R1 dagster
Branch
prha/schedule_page
Lint
Lint OK
Unit
No Unit Test Coverage

Event Timeline

This is great!

js_modules/dagit/src/schedules/ScheduleRow.tsx
86–92

Could you explain what match is doing here?

This revision is now accepted and ready to land.Dec 3 2019, 12:23 AM
sashank requested changes to this revision.Dec 3 2019, 12:27 AM

From Slack: Would be nice for the 5th column in the top bar to show attempts instead of runs, so that the colors match up with the attempts list

This revision now requires changes to proceed.Dec 3 2019, 12:27 AM
js_modules/dagit/src/schedules/ScheduleRow.tsx
86–92

yeah, match is non-null if it matches the /schedule/:scheduleName route.

We check it so that we don't show a link if we're already on the page that we're linking to.

updated to display attempts rather than runs

Looks great! Just two minor suggestions

js_modules/dagit/src/schedules/ScheduleRow.tsx
179

What do you think about wrapping this in a Tooltip too that reads "Click to show error", just for consistency

<Tooltip
  position={"top"}
  content={"Click to show error"}
  wrapperTagName="div"
  targetTagName="div"
>
  <AttemptStatus status={attempt.status} />
</Tooltip>
python_modules/dagster-graphql/dagster_graphql/schema/run_schedule.py
154 ↗(On Diff #7063)

[1]

198 ↗(On Diff #7063)

Maybe we should extract this and [1] to a function so that paths don't get out of sync

This revision is now accepted and ready to land.Dec 3 2019, 9:04 PM
This revision was automatically updated to reflect the committed changes.