[dagit] Add more useful things to Pipelines list
ClosedPublic
Actions

Authored by dish on Feb 22 2021, 7:19 PM.

Details

Reviewers

bengotow
prha
dgibson

Commits

R1:3a3d714a757d: [dagit] Add more useful things to Pipelines list

Summary

Add some more useful features to the all-pipelines view at /pipelines.

Fuzzy search
Schedules, sensors, and recent runs
Show repo if more than one repo in the workspace, otherwise just don't show it

Reuse the table for the repo-level view as well (/workspace/foo@bar/pipelines).

Test Plan

View /pipelines and repo-level pipelines views. Verify that list looks good and filters nicely with search. Verify that empty states look correct.

Diff Detail

Repository

R1 dagster

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

dish created this revision.Feb 22 2021, 7:19 PM

Screen Shot 2021-02-22 at 1.17.17 PM.png (1×2 px, 382 KB)

Screen Shot 2021-02-22 at 1.17.03 PM.png (1×2 px, 174 KB)

Harbormaster completed remote builds in B26278: Diff 32102.Feb 22 2021, 7:26 PM

dish requested review of this revision.Feb 22 2021, 7:26 PM

This looks great to me! It does seem it'd be nice to have the fuse implementation pulled out into a helper hook because it gets a little technical, but there may not be anywhere else that we could re-use it so it's probably not worth factoring apart into two pieces.

js_modules/dagit/src/workspace/AllPipelinesRoot.tsx
61	This looks good to me but it's becoming quite the mouthful - since this all boils down into a flat object using these keys, it might be a bit more readable to create a local var and then assign values into it in three nested for loops? I think this would be more confusing if we didn't have a return type declaration that makes it easy to mentally ignore the whole thing, though.
64	I'm not sure this case is ever true?
js_modules/dagit/src/workspace/RepositoryPipelinesList.tsx
60	Whoops 😅

This revision is now accepted and ready to land.Feb 22 2021, 8:02 PM

It does seem it'd be nice to have the fuse implementation pulled out into a helper hook because it gets a little technical, but there may not be anywhere else that we could re-use it so it's probably not worth factoring apart into two pieces.

Yeah, I'm going to keep an eye on this. Right now we'll just have this and the global search behavior using Fuse, but it could end up being more.

js_modules/dagit/src/workspace/AllPipelinesRoot.tsx
61	Yeah, it turned out to be a lot of `reduce` 😆 Since it's a return value for a `React.useMemo` anyway, I can see about trying to simplify things.
64	Good point