We never actually use the step_keys_to_execute_paramater on create_run_for_pipeline in any significant way, and removing it allows for singificant code simplification.
Details
Details
switched around unit tests where necessary.
Diff Detail
Diff Detail
- Repository
- R1 dagster
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| python_modules/dagster/dagster/core/instance/__init__.py | ||
|---|---|---|
| 718–724 | can totally get rid of this, I have that coming in a follow up with a larger treatment of step_keys_to_execute in general. | |
| python_modules/dagster/dagster/core/instance/__init__.py | ||
|---|---|---|
| 714–724 | yeah im confused by whats happening here - would this make more sense as part of the memoization block above ? Its not clear to me why were always doing this check | |
Comment Actions
gonna round trip this one since im a little weary of that new check run for all creates
| python_modules/dagster/dagster/core/instance/__init__.py | ||
|---|---|---|
| 714–724 | Right - so there's some serious weirdness here. Essentially, execution_plan.step_keys_to_execute does not have the same meaning as step_keys_to_execute that we pass around independently. In the step_keys_to_execute argument that we independently pass around, None means every step. But execution_plan.step_keys_to_execute does not accept a None value, and instead every step means every step. There are two routes, as I see it. The less-friction one is what I have done above, essentially translating execution_plan.step_keys_to_execute back into the freeformed one with the None assumption. The other one would be to make execution_plan.step_keys_to_execute conform to the way we use it free-standing. At the very least, I'm pretty sure that we need to keep the None == ever step assumption around for the free-standing one, so we don't break people's run history. Does that somewhat clear it up / do you have thoughts on which approach you prefer? | |