This diff restructures the reexecution flow to use previous_run_id and the existing step_key arguments on ExecutionParams instead of a separate ReexecutionConfig. It moves the step_keys_to_execute onto ExecutionPlan to thread through the APIs and engine. Adds a RetryMemoization class that handles the intermediate management across runs.
schrockn alangenfeld max
- Group Reviewers
- R1:adffd3ff370f: Remove reexecution_config, add memoization strategy for reexecution
This is a huge improvement. My questions/concerns are mostly around the memoization strategy class.
If there are "missing steps" which is the execution plan ever constructed?
so much better
manage_intermediates is a bit vague. had to read the function to understand why you named it as such.
I'm actually having a hard time following the function. i think a docblock describing the logic could be very useful.
a comment or encapsulating this (this being what is in the if statement) in function with a descriptive name could be more clear
I think we should resolve the TODO before we merge. This could easily become cost-prohibitive.
my last hang up is if this name sufficiently communicates whats going to happen if you set it. Some options would be a better name, a sibling arg, or a wrapper arg to communicate that this implies that we are retrying based on the results of said previous run.
Yeah, I erred on the side of just making the GraphQL API descriptive (e.g. "here is the previous run id"), allowing for different future behaviors in the memoization strategy (making it pluggable).
I feel comfortable renaming this retryRunId or something like that until we have a better sense of what those other behaviors look like.
why is this a separate function?
per slack thread let's just make this into a single function copy_required_intermediates_for_execution or something similar
can we just put this into init?
or indeed parentRunId? but actually i don't think i understand the difference between retryRunId and previous_run_id
it might be nice to print the name or class of the intermediates manager here to aid debugging