Page MenuHomePhabricator

Fixes for Airflow execution with DagsterInstance
ClosedPublic

Authored by max on Thu, Sep 12, 2:26 AM.

Details

Reviewers
alangenfeld
Group Reviewers
Restricted Project
Commits
R1:c0b04f6e4e5a: Fixes for Airflow execution with DagsterInstance
Summary

make_airflow_dag now takes an optional DagsterInstance argument

Test Plan

Unit

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

max created this revision.Thu, Sep 12, 2:26 AM
max updated this revision to Diff 4662.Thu, Sep 12, 5:48 PM

Rebase

alangenfeld requested changes to this revision.Thu, Sep 12, 6:32 PM
alangenfeld added inline comments.
js_modules/dagit/src/schema.graphql
181–186

does this type just have a bad name or do we need to update ExecutePlanFailure as well

python_modules/dagster-airflow/dagster_airflow/factory.py
66

add comment probably

python_modules/dagster-airflow/dagster_airflow/operators/docker_operator.py
88–90

remove

144

should this really be optional? [1]

209

[1] does None actually work here?

python_modules/dagster-airflow/dagster_airflow/operators/python_operator.py
59–60

this isnt obvious to me why this behavior makes sense - add a comment?

python_modules/dagster-airflow/dagster_airflow/operators/util.py
12–28

just write two methods - not a fan of this bool stuff

python_modules/dagster-graphql/dagster_graphql/client/mutations.py
135–136

same - refactor to avoid bool

This revision now requires changes to proceed.Thu, Sep 12, 6:32 PM
max updated this revision to Diff 4669.Thu, Sep 12, 6:55 PM

Rebase

max added inline comments.Thu, Sep 12, 8:24 PM
js_modules/dagit/src/schema.graphql
181–186

this type is poorly named and also defined in a weird place

python_modules/dagster-airflow/dagster_airflow/operators/docker_operator.py
88–90

this is used in the docker operator code we're extending

209

yes, because if no instance is set we don't attempt to call .get_or_create_run() or .handle_new_event()

alangenfeld accepted this revision.Thu, Sep 12, 8:44 PM
alangenfeld added inline comments.
python_modules/dagster-airflow/dagster_airflow/operators/docker_operator.py
88–89

*

python_modules/dagster-graphql/dagster_graphql/client/mutations.py
67

comment about what raw means or name better (not sure what tho)

This revision is now accepted and ready to land.Thu, Sep 12, 8:44 PM