HomeElementl

step-skipping bug - remove StepExecutionSkipped

Description

step-skipping bug - remove StepExecutionSkipped

Summary:
This diff removes the StepExecutionSkipped flag and defers to the airflow+docker call site to tell if they should skip a step -- In D4997, we introduced StepExecutionSkipped, a serializable object (not a dagster event). The dagster api execute_step CLI will output it to signal if we need to skip a step.

Problem:
https://dagster.slack.com/archives/C014N0PK37E/p1616620216068400
with a k8s+celery executor, a user hit:

Execution of pipeline "my_pipeline" failed. An exception was thrown during execution.
AttributeError: 'StepExecutionSkipped' object has no attribute 'is_step_failure'
Stack Trace:
  File "/opt/pysetup/.venv/lib/python3.8/site-packages/dagster/core/execution/api.py", line 754, in pipeline_execution_iterator
    if event.is_step_failure:
An exception was thrown during execution that is likely a framework error, rather than an error in user code.
AttributeError: 'StepExecutionSkipped' object has no attribute 'is_step_failure'
Stack Trace:
  File "/opt/pysetup/.venv/lib/python3.8/site-packages/dagster/grpc/impl.py", line 76, in core_execute_run
    yield from execute_run_iterator(recon_pipeline, pipeline_run, instance)
,  File "/opt/pysetup/.venv/lib/python3.8/site-packages/dagster/core/execution/api.py", line 829, in __iter__
    yield from self.iterator(
,  File "/opt/pysetup/.venv/lib/python3.8/site-packages/dagster/core/execution/api.py", line 754, in pipeline_execution_iterator
    if event.is_step_failure:

Diagnosis:
bc we are parsing all the info sent back as dagster events, when dagster api execute_step outputs this non-dagster-event StepExecutionSkipped, the core path will break as above.

The root problem is the normal execution flow on the dagster api execute_step should be flagging this at all. This should only be hit by the airflow + docker case - because each dagster step maps to an airflow task and airflow+docker uses the execute_step cli path, i.e. skipping a step means skipping an airflow task, so this case doesn't have a run-level plan process to control the skipping logic.

Test Plan: bk

Reviewers: alangenfeld, sandyryza

Reviewed By: alangenfeld

Differential Revision: https://dagster.phacility.com/D7133

Details

Provenance
yuhanAuthored on Mar 25 2021, 12:10 AM
Reviewer
alangenfeld
Differential Revision
D7133: step-skipping bug - remove StepExecutionSkipped
Parents
R1:7e03618f1801: remove deprecated examples
Branches
Unknown
Tags
Unknown