Page MenuHomeElementl

DynamicOutput re-execution

Authored by alangenfeld on Dec 15 2020, 11:02 PM.



Steal alot of @catherinewu's work from D4936 to support re-execution of dynamic output pipelines. Python API and GraphQL.

Test Plan

added tests

Diff Detail

R1 dagster
Lint Not Applicable
Tests Not Applicable

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Dec 15 2020, 11:22 PM
Harbormaster failed remote builds in B22918: Diff 27879!
Harbormaster returned this revision to the author for changes because remote builds failed.Dec 16 2020, 12:15 AM
Harbormaster failed remote builds in B22924: Diff 27885!

Assuming this depends on D5542?

Unfortunately, the re-execution code paths are a bit messy.. There are a few ways to re-execute a pipeline:

  1. Python API reexecute_pipeline
  2. retry in graphql (the retry from failure button in dagit) [1]
  3. other re-execution options in dagit (i.e. "selection" and "from selected") [2]

In case you missed the case 3, in which the re-execution logic in dagit in JS land (where the code paths get messy :/) we should test if all the re-execution buttons still work - at least test the executionMetadata.stepKeys in graphql tests


[1] covered in get_retry_steps_from_execution_plan ✅


[2] doubt if this would work for dynamic pipelines

This comes from where the step key transformation happens in toGraphQueryItems .


related to [2] maybe we can add a retry_two to test executionMetadata.stepKeys

Very concerned about using the step key DSL in _update_tracking_dict and friends. I think should be persisting the structured step key information DagsterEvent to reduce the surface area of our reliance on the DSL


oof really don't like from_key calls in such a critical path. It looks like this also encodes the pseudo step key DSL to be persisted on DagsterEvent for all of time


This is pretty terrifying. I think we need to document this and test it more carefully

This revision now requires changes to proceed.Dec 21 2020, 4:05 PM

rebase, adding step_handle to DagsterEvent shown in D5755

Very happy with the tracking changes

Will let @yuhan do finally approval to sync with ObjectStoreOperation stuff


change to map


complicated enough where extracting it into its own function and having tests to document would be useful

overall looks good to me. but the .map seems be the cause failing the tests

This revision is now accepted and ready to land.Jan 9 2021, 12:54 AM
This revision was automatically updated to reflect the committed changes.