Page MenuHomeElementl

[dagit] Add more useful things to Pipelines list
ClosedPublic

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

Details

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 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

Nested forEach more legible than nested reduce

This revision was automatically updated to reflect the committed changes.