Page MenuHomePhabricator

[trigger-6] add execution-time based graphs on the trigger dashboard
AbandonedPublic

Authored by prha on Aug 28 2020, 8:28 PM.

Details

Test Plan

bk

Diff Detail

Repository
R1 dagster
Lint
Lint OK
Unit
No Unit Test Coverage

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Aug 28 2020, 8:43 PM
Harbormaster failed remote builds in B17764: Diff 21580!
prha requested review of this revision.Aug 28 2020, 9:50 PM
js_modules/dagit/src/TriggerGraphs.tsx
121

stepStat && stepStat.endTime && stepStat.startTime or even stepStat && stepStat.stepKey && stepStat.endTime && stepStat.startTime ?

129

nit: should we standardize getPipelineExpectationSuccessForRun and getStepMaterializationCountForRun to either both fetch from PipelineRunStatsSnapshot or both sum up from the step level counts? I guess the former would require adding expectationsSucceeded and expectationsFailed to PipelineRunStatsSnapshot. :shrug:

203

nit: => arr.reduce((a,b) => a + b, 0), maybe reduce could be less readable

js_modules/dagit/src/TriggerGraphs.tsx
121

Unnecessary, but will type to make it clear that stepStat is not false-y.

129

:shrug:

203

I tend to not like reduce as much, as force of habit. In this case, not that bad.

This revision is now accepted and ready to land.Sep 9 2020, 3:25 PM