Page MenuHomeElementl

Present pipeline+mode as a tuple, lock pipeline mode in the playground
ClosedPublic

Authored by bengotow on May 24 2021, 10:37 PM.

Details

Summary

This diff adds a feature flag that presents each pipeline+mode as a distinct item in the left nav and locks the mode selection within the pipeline tab.

The pipeline mode now appears in the URL all the time after the pipleine name, and I updated the helpers and convenience methods so the pipeline URLs are built in fewer places.

Todo:

  • The quicksearch UI still shows individual pipelines
  • The repo pipeline list still shows individual pipelines

Test Plan

Run tests

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

  • Add support to the repo pipeline list

The pipeline mode now appears in the URL all the time after the pipleine name

Do old links still work? Does this effect the existing experience in other ways?

Hey @alangenfeld! Let me double check that the URLs without the mode work as expected - we can definitely maintain backwards compatibility. The only other change in this diff that isn't feature-flagged is that places where both pipeline mode and snapshot are listed (I believe just the run details header) have changed from pipeline@snapshot:mode to pipeline:mode@snapshot. I think the old style was chosen because the snapshot reflects the pipeline and then mode applies configuration, but I think the entire thing should be clickable and pipeline:mode go to the pipeline overview page and @snapshot goes to the snapshot-in-time view of the pipeline. Depending on how long we want to leave this feature flagged it could make sense to make that conditional as well but it seemed minor.

The only other change in this diff that isn't feature-flagged is that places where both pipeline mode and snapshot are listed (I believe just the run details header) have changed from pipeline@snapshot:mode to pipeline:mode@snapshot

Agree this seems like a fine change

would like @dish to clear this since its pretty substantial

dish added inline comments.
js_modules/dagit/packages/core/src/nav/RepositoryContentList.tsx
160

Might be good to put this back into the useMemo so that we don't have to run it every render

This revision is now accepted and ready to land.Jun 1 2021, 10:02 PM
bengotow added inline comments.
js_modules/dagit/packages/core/src/nav/RepositoryContentList.tsx
160

Ahh that seems like a good call! Will fix. I think there are some other places I should probably drop these... there are some much worse offenders in the viz components and I always forget this is a pretty easy add... 🙈

  • Apply diff feedback
  • Use a React hook to upgrade old urls to include mode, wrap no-snapshot-id behavior in the same way
This revision is now accepted and ready to land.Jun 3 2021, 5:38 AM
  • Test fixes
  • Merge branch 'master' of github.com:dagster-io/dagster into abg/pipelineandmode