Page MenuHomePhabricator

[dagit] Separate snapshot and current pipeline permalinks
ClosedPublic

Authored by dish on Wed, Oct 7, 8:40 PM.

Details

Summary

Separate a specific snapshot view from the main view of a pipeline. The snapshot permalink will have two tabs:

  • Definition, for the state of the snapshot
  • Runs, for runs specific to that snapshot ID

It will also display "Historical snapshot" or "Current snapshot" tags, depending on whether the viewed snapshot ID is the current one for the pipeline.

Breadcrumbing allows navigation back to the main pipeline page, where one can use the Playground and view all runs for the pipeline.

Stacked on top of the change that modifies how the Runs table rows are rendered. Screenshots below.

Test Plan

View all existing pipeline@snapshot paths, verify that they display the correct breadcrumbs, tag, and tabs.

View pipeline-only paths, verify same.

Diff Detail

Repository
R1 dagster
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Main pipeline view. Same as before.

Runs for current snapshot:

Current snapshot, note only "Definition" and "Runs" tabs:

Historical snapshot definition page, note yellow tag and only "Definition" and "Runs" tabs:

dish requested review of this revision.Wed, Oct 7, 8:46 PM

Redirects for paths that don't work for snapshots

I feel not obvious/easy to get back to the playground/overview pages if you end up on a snapshot page, especially since we use the same tabs UI for both ["Definition, "Runs"] and ["Overview", "Definition", "Runs", "Partitions"].

js_modules/dagit/src/App.tsx
73

Does this affect loading an empty workspace?

I feel not obvious/easy to get back to the playground/overview pages if you end up on a snapshot page, especially since we use the same tabs UI for both ["Definition, "Runs"] and ["Overview", "Definition", "Runs", "Partitions"].

My thought was that the breadcrumb would allow going back to the main pipeline page to see the overview and playground, but it's helpful to know that that's not clear.

Multiple levels of tabs might work, so that the main pipeline tabs would always be visible.

js_modules/dagit/src/App.tsx
73

Ah, good question. I'll try that.

Multiple levels of tabs might work, so that the main pipeline tabs would always be visible.

This ended up being a little overwhelming, it's too many tabs. :(

Remove empty repo conditional, doesn't seem necessary

alangenfeld added a subscriber: alangenfeld.

i like the overall direction - will leave to those with stronger opinions for final choices

This looks great! I think the new breadcrumbs approach is a lot more clear. Just left a few inline comments but from a code / usability standpoint I think this is a win.

js_modules/dagit/src/partitions/PipelinePartitionsRoot.tsx
29

rm!

js_modules/dagit/src/runs/RunTable.tsx
167

Do we have a PipelinePathUtils helper that will go from run => pipeline path? It'd be slightly nice to pull this out into a helper so the parsing in both directions is in the same file. (Though admittedly also silly for just an @ character, do not feel strongly about this!)

This revision is now accepted and ready to land.Tue, Oct 13, 3:27 PM
js_modules/dagit/src/partitions/PipelinePartitionsRoot.tsx
29

Oh man, I should make this fail the build...

js_modules/dagit/src/runs/RunTable.tsx
167

Good point, I feel like this probably should exist -- will take a look, and add it if not.

dish retitled this revision from [dagit] RFC: Separate snapshot and current pipeline permalinks to [dagit] Separate snapshot and current pipeline permalinks.Tue, Oct 13, 5:50 PM