Page MenuHomePhabricator

Improve display of materializations and expectations
ClosedPublic

Authored by max on May 20 2019, 9:59 PM.

Details

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

max created this revision.May 20 2019, 9:59 PM
max edited the summary of this revision. (Show Details)May 20 2019, 10:00 PM
max updated this revision to Diff 845.May 20 2019, 10:34 PM

Add tests

max updated this revision to Diff 846.May 20 2019, 10:36 PM

Black

Per discussion, worth compacting the display of materializations to one line, and perhaps including the path (this also gives a quick visual indication if something has been written locally vs. to S3)

alangenfeld added inline comments.
js_modules/dagit/src/DisplayEventsContainer.tsx
78–81

?

bengotow accepted this revision.May 21 2019, 11:58 PM

This looks great to me! Nice visual improvement. It's definitely less generic if we break down "display events" into materializations + expectations and pass those two categories in explicitly, but that does allow for a more customized presentation of each type. I think an alternative would be to allow for display events to have children and do the disclosure triangles in a super generic way, but given that these are probably the two major categories it seems fine to just handle each one.

We should verify that we're OK with users not being able to throw arbitrary "display events" in that don't fall into one of the two categories.

js_modules/dagit/src/ExecutionPlanBox.tsx
176

Hah wow - can't believe I hadn't noticed this!

This revision is now accepted and ready to land.May 21 2019, 11:58 PM

Yeah I think it would be cool to collapse items that only have one key value pair to a single line. But we can do that as a follow on.

max added a comment.May 22 2019, 8:51 PM

@bengotow I trust your instincts and am happy to follow your lead on making the disclosure triangle stuff more generic -- I'm not super confident in how I carve up views into components.

js_modules/dagit/src/DisplayEventsContainer.tsx
78–81

Thx, I'll pull it out

max updated this revision to Diff 925.May 22 2019, 9:17 PM

Rebase

max updated this revision to Diff 929.May 22 2019, 9:40 PM

Rebase