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
Branch
prha/triggers
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