Page MenuHomePhabricator

Fix Airflow skip behavior
ClosedPublic

Authored by natekupp on Jul 30 2019, 4:25 AM.

Details

Reviewers
alangenfeld
Group Reviewers
Restricted Project
Commits
R1:2ffae4c0d9d5: Fix Airflow skip behavior
Summary

Turns out that marking TaskInstance objects as skipped doesn't work in Airflow proper, because the DagRun overwrites the task state after user code execution.

Instead, we do the Python exceptions-as-control-flow thing and use AirflowSkipException, this appears to be the canonical way to implement skipping-yourself behavior: https://blog.godatadriven.com/zen-of-python-and-apache-airflow

Test Plan

unit and hand-tested in Airflow

Diff Detail

Repository
R1 dagster
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

natekupp created this revision.Jul 30 2019, 4:25 AM
natekupp edited the test plan for this revision. (Show Details)Jul 30 2019, 4:35 AM
natekupp edited the summary of this revision. (Show Details)Jul 30 2019, 4:41 AM
natekupp added a reviewer: Restricted Project.Jul 30 2019, 4:46 AM
alangenfeld requested changes to this revision.Jul 30 2019, 3:02 PM
alangenfeld added a subscriber: alangenfeld.
alangenfeld added inline comments.
python_modules/dagster-airflow/dagster_airflow/test_fixtures.py
31

what is the usual return type of task execution? worth hoisting this up as SKIPPED_STEP_MARKER or something to make it clear what this string is

This revision now requires changes to proceed.Jul 30 2019, 3:02 PM
natekupp added a subscriber: max.Jul 30 2019, 6:09 PM
natekupp added inline comments.
python_modules/dagster-airflow/dagster_airflow/test_fixtures.py
31

I switched this to just pass along the exception value instead of using a magic string per discussion w/ @max

This revision is now accepted and ready to land.Jul 30 2019, 6:11 PM
This revision was automatically updated to reflect the committed changes.