Page MenuHomePhabricator

Repository-level solids explorer + graphQL root
ClosedPublic

Authored by bengotow on Oct 24 2019, 6:47 PM.

Details

Reviewers
alangenfeld
Group Reviewers
Restricted Project
Commits
R1:1a9b0a09e7fb: Repository-level solids explorer + graphQL root
Summary

This diff adds the graphql interface proposed by Alex in his feedback on D365. A new root query allows you to fetch solids and their invocations across the whole repo.

I think that having more repo-level concepts / pages in Dagit will inform how we want the new nav to work. This diff adds a solid explorer tab alongside Schedule and Runs. This view allows you to view all of the solids and see the same definition details visible when you click them in the Pipeline Explore tab. (I refactored a bunch of stuff to make our components more explicit about whether they require a definition, an invocation, or both, allowing the SVG and sidebar stuff to be reused.)

So far the coolest part of this UI is that you can filter solids by pipeline OR by data types. So I can click a solid with a "DataFrame" output and see all the solids that have a "DataFrame" input.

I think the actual design could use a bit of iteration, but curious to get thoughts on this v1!

Screenshots:


Other Concept Idea - this seemed less practical than a basic alphabetical list, but it looks slick ๐Ÿ˜… (Ignore the fact there are some duplicates)

Test Plan

Will add a snapshot

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

bengotow created this revision.Oct 24 2019, 6:47 PM
bengotow edited the summary of this revision. (Show Details)Oct 24 2019, 6:49 PM
bengotow edited the summary of this revision. (Show Details)

do you want review on the code yet or just feedback on the designs?

Hey! I think if the design feedback is minor it's ready for a full review - if we end up changing the UI we can hold off on that, but I'd love to get your eyes on the python side now. I pieced together the graphql resolver a bit and that stuff is probably not ideal.

bengotow updated this revision to Diff 5998.Oct 24 2019, 7:37 PM

Rebase and fix black

yeah this is pretty amazing. any feedback i have are about existing components which I believe we can address in followups.

Namely:

  1. We need to work on usability of the typeahead. Right now to do anything useful it takes two steps and it's easy to screw up.
  2. Could work on information density of the definitions and making invocations more prominent.

But this is awesome.

The solid def grid is cute but I think we should stick to the list

max added a subscriber: max.Oct 25 2019, 12:24 AM

oh, this is awesome

alangenfeld added inline comments.Oct 25 2019, 8:12 PM
js_modules/dagit/src/SidebarSolidDefinition.tsx
230โ€“246

nit: might be a bit cleaner to resolve the shown invocations and remaining count in one place above this return stmt

js_modules/dagit/src/SidebarSolidHelpers.tsx
165โ€“177

facemelt

js_modules/dagit/src/schema.graphql
825โ€“996

I think we could do a little better with names, some ideas:

used_solids: [UsedSolid!]!

type UsedSolid {
  definition: ISolidDefinition!
  invocations: [SolidInvocation!]!
}

type SolidInvocation {

maybe SolidInvocationSite, SolidInvocationLocation

js_modules/dagit/src/solids/SolidsRoot.tsx
142โ€“152

now i need to actually learn how these things works

letsgetweird

python_modules/dagster-graphql/dagster_graphql/schema/roots.py
186โ€“188

nit: you can use defaultdict to clean this pattern up

alangenfeld requested changes to this revision.Oct 28 2019, 3:52 PM

I think theres enough feedback here to warrant a respin / tidy up pass

This revision now requires changes to proceed.Oct 28 2019, 3:52 PM
alangenfeld added inline comments.Oct 28 2019, 6:39 PM
js_modules/dagit/src/solids/SolidsRoot.tsx
142โ€“152

should we pass the dependency list arg to useEffect I can't quite decipher easily how often this will re-render where we may not want to run this side effect

bengotow planned changes to this revision.Oct 29 2019, 7:18 PM
bengotow added inline comments.
js_modules/dagit/src/SidebarSolidHelpers.tsx
165โ€“177

Hahaha I'll see if our webpack config will let me import this from a file - this doesn't do anything with it's props so it could be an actual .svg resource I think...

js_modules/dagit/src/schema.graphql
825โ€“996

Hmm yeah this could definitely be better - I'm not sure about UsedSolids because solids don't really exist in the repo until they're used, so it's impossible to have an unused solid? Hmm - maybe I'm overthinking it...

the only other thing I can think of is

solid_collection: [SolidCollectionItem!]!

type SolidCollectionItem {
  definition: ISolidDefinition!
  invocations: [SolidInvocationSite!]!
}

and I don't really like that better. Will change this to UsedSolid + SolidInvocationSite for now.

js_modules/dagit/src/solids/SolidsRoot.tsx
142โ€“152

So in this case I'm using React.useEffect without a dependency list, so this function runs after every render. (same behavior as the old componentDidUpdate method). Both of these if statements essentially enforce state consistency so I think we want/don't-mind them running on every pass.

(Should add that the typeExplorer check here is something I'll probably revisit in another diff but I didn't want to make this one even bigger - right now clicking a type adds ?typeExplorer=<displayName> to the URL bar, essentially assuming you're looking at the Explore tab. We'll want to use React context to decouple the click from the URL a bit more I think.

python_modules/dagster-graphql/dagster_graphql/schema/roots.py
186โ€“188

Oh nice! Did not know about defaultdict... this is great!

bengotow updated this revision to Diff 6063.Oct 29 2019, 8:27 PM
  • Address diff feedback
  • Add solid explorer snapshot test, fix other tests, rebase
This revision is now accepted and ready to land.Oct 29 2019, 8:34 PM
bengotow updated this revision to Diff 6068.Oct 29 2019, 9:04 PM
  • Make ordering of solids in usedSolids query deterministic
This revision was automatically updated to reflect the committed changes.