Page MenuHomePhabricator

Clean up partitions UI and improve backfill usability
ClosedPublic

Authored by bengotow on Oct 19 2020, 5:32 AM.

Details

Summary

Hey folks! I've made a few high level changes to the partition UI. I think there are more to come, but I'd love to get everyone's thoughts. This diff:

  • Compacts the partition headers and options to match my earlier mockups:
    • Makes the partition picker more identical to the one on the Runs tab
    • Moves the 7, 30, 120 day options up to make it clear they're page-wide settings
    • Moves Launch Partition Backfill to a standard looking button
  • Reworks the partition backfill UI to reflect our recent conversations:
    • You can now specify partitions by typing into a freeform text box. I tried a couple different UIs that let you put in an A-B range OR select items individually and it all felt painful. I think that letting people type (and copy/paste) in addition to using the preview to make their selections might be ideal.
      • 2020-04-* to specify all matching partitions
      • [2020-01-01...2020-04-04] to specify ranges
      • If you select in the UI we generate this compact representation for you
    • I moved the step subset up out of the visualization into the "backfill setup form" so that it's more clear that the filter is actually a piece of the configuration.
    • Instead of hiding options that aren't available they're disabled so you know Dagster has cool features you aren't using!
    • There's now a tooltip explaining the drag-to-select behavior

Thoughts:

I would really like to make the Launch Backfill button jump you to a new page, or open a modal, but I feel like the current approach (outlined form) might be more usable because it allows you to work with the context / data on the page while you configure your backfill? Would be interested to know what everyone thinks.

It appears that the partition backfill UI does allow you to use partitions with no previous runs, as long as you don't select "from last run" and don't need step subsets. I think we can enable the step subset box without being in re-execution mode?



Test Plan

Run tests

Diff Detail

Repository
R1 dagster
Branch
dbg/backfill-step-subset
Lint
Lint OK
Unit
No Unit Test Coverage

Event Timeline

bengotow edited the summary of this revision. (Show Details)
prha requested changes to this revision.Oct 19 2020, 1:16 PM

Still doesn't seem possible to set subset selection for the partitions with no runs. Is that going to be part of this diff?

js_modules/dagit/src/partitions/PartitionsBackfill.tsx
255

maybe only call partitionsToText on blur, but o.w. dynamically change the selected state?

283–289

Can we add some sort of usage placeholder text? It took a while to figure out how to interact with this textbox.

Also, it's strange that we only call setSelected on blur, instead of onChange when we've just added a delimiter (e.g. comma). Made it way harder to figure out what the right syntax was.

294

:)

This revision now requires changes to proceed.Oct 19 2020, 1:16 PM

Thanks for the review! Will add the subset w/o previous runs logic changes. Just needed to re-read Sandy's doc now πŸ‘

js_modules/dagit/src/partitions/PartitionsBackfill.tsx
283–289

Ahh definitely - I added a dynamic placeholder. I think we can also make this update on delimiter, but right now it doesn't store your exact text so it's possible it will revise the expression when you type ,... Will see if I can fix if Nick is a fan of this approach!

Simplify state a bit - coerce into a valid state when you make changes so enabled/disabled state of various features don't need to reference many vars

Possibly crazy idea: can we use a yaml CodeMirror here? E.g.

- '2020-04-20'
- start: '2020-04-10'
  end: '2020-04-17'
- start: '2020-02-12'
  end: '2020-02-17'

Then you might be able to depend on yaml validation instead of manual validation, and force the document to conform to an array of strings and {start, end} objects (or tuples). Syntax highlighting might be a nice plus too.

Either way, I'm very much in favor of having some tests on syntax-to-state and vice versa. :)

I kind of like the codemirror idea... and having yaml removes the requirement to learn another DSL.

js_modules/dagit/src/partitions/PartitionsBackfill.tsx
335

I think we can probably enable this to unblock prezi (step subselection without previous run). I'll create a follow-up task to track checking uncovered inputs or making DagsterStepOutputNotFoundError have a less misleading error message.

Ahh man, yeah I suppose this could use YAML. I personally prefer the ... syntax because it encodes that the range is inclusive of the end, but that's not hard to see from the live preview. We'll need to reflow the UI a bit because that'll take up a lot more vertical space.

Also, if we use YAML I wonder if we YAML-encode the rest of the options as well so you can paste an entire backfill config? The only thing is that YAML is largely undiscoverable without a bunch of our surrounding UI, and if we take it too far people will definitely confuse it for config editing.

bengotow added inline comments.
js_modules/dagit/src/partitions/PartitionsBackfill.tsx
335

Yep I think we can enable this easily, will update this diff now πŸ‘

  • Enable subset selector without previous runs

I think the textarea could be a little beginner-friendly in terms of the syntax, but we can iterate and see what folks think

This revision is now accepted and ready to land.Oct 20 2020, 9:04 PM