Page MenuHomePhabricator

[dagit] Instance paths for runs and snapshots
ClosedPublic

Authored by dish on Tue, Nov 3, 3:13 PM.

Details

Summary

Continuing the Dagit URL namespacing transition.

  • /instance/runs/:runId for all individual run pages
    • This includes a change to the top nav on this page, to show the pipeline name (linked) and the run ID, as well as the run status tag
  • /instance/snapshots/:pipelinePath for all pipeline snapshots
    • Basically the same as the current pipeline page, but separated into its own route tree

After this, I have a larger change that will move workspace-level objects to /workspace URLs, as described in the Quip, and changes the behavior of repository context within Dagit. That should be the last major change, and I'll clean things up afterward.

Test Plan

Open Dagit, view different sections of the app:

  • Runs, verify that run links and snapshot links point to the new paths
  • Perform a run execution, verify redirect to /instance/runs/... path
  • Use tabs on Snapshot page, verify correct behavior
  • Click the pipeline name on a snapshot page, verify redirect (this will be a better workspace-specific redirect in a followup)

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

Harbormaster returned this revision to the author for changes because remote builds failed.Tue, Nov 3, 3:21 PM
Harbormaster failed remote builds in B20724: Diff 25121!
dish requested review of this revision.Tue, Nov 3, 5:22 PM
prha added inline comments.
js_modules/dagit/src/workspace/WorkspacePipelineDisambiguationRoot.tsx
55–59

clean up?

This revision is now accepted and ready to land.Tue, Nov 3, 11:42 PM
dish planned changes to this revision.Wed, Nov 4, 12:59 AM

I broke a handful of things here, will fix.

Fix paths on Pipeline explorer pages

This revision is now accepted and ready to land.Thu, Nov 5, 7:48 PM
js_modules/dagit/src/workspace/WorkspacePipelineDisambiguationRoot.tsx
55–58

Keeping this here as a reminder -- I'm going to switch over to use workspace-namespaced paths in the followup.

dish requested review of this revision.Thu, Nov 5, 7:52 PM

Re-requesting review as a sanity check.

prha added inline comments.
js_modules/dagit/src/snapshots/SnapshotNav.tsx
99–105

nit: may want to eventually sync this with the tab definition by building it from the tabs list:

tabs.find(x => x.title === activeTab)?.title
This revision is now accepted and ready to land.Mon, Nov 9, 5:18 PM
This revision was landed with ongoing or failed builds.Mon, Nov 9, 7:36 PM
This revision was automatically updated to reflect the committed changes.