Page MenuHomePhabricator

extract run graph utils from partition-specific graphs

Authored by prha on Sep 16 2020, 12:25 AM.



Based off of D4307, which created graphs for non-partitioned pipelines, using execution time as the X-axis

Test Plan

Rendered partition dashboard, schedule dashboard, saw rendered graphs

Diff Detail

R1 dagster
Lint OK
No Unit Test Coverage

Event Timeline

prha requested review of this revision.Sep 16 2020, 12:43 AM

This looks great! I added a few inline comments but I like the idea of creating a base class for these graphs, the chartJS stuff is pretty verbose and it's always a bummer to have a ton of chart config in the middle of a component.


It might be nice to document in a comment up here that the Y value within each PointValue is freeform, but the X value needs to be the start time of the run because of the logic in _fillPartitions.

It might also be nice to export the _getExecutionTime method as something more opaque (eg: getXForRun(run)) so that anyone who adds another graph using this component uses the exact same code for creating X values.


I added the chart.js types to the project so we should be able to make this return an instance of ChartOptions - if it's not too much of a pain to add here, I think it'd be nice to be able to cmd-click through to their types to investigate these options.

(If it complains about scales you could say const scales: ChartOptions['scales'] = to reference the type of the object member in the other type.)


This is a really dumb nit but it might be better to call this _getRunStartTime just to avoid introducing another word with an equivalent meaning - though I like execution time better than start time ...

This revision is now accepted and ready to land.Oct 5 2020, 7:55 PM