Page MenuHomePhabricator

Remove SynchronousExecutionManager completely
AbandonedPublic

Authored by max on Sep 5 2019, 11:54 PM.

Details

Reviewers
schrockn
alangenfeld
natekupp
Group Reviewers
Restricted Project
Summary

This does add some overhead to any execution paths where we invoke the dagster-graphql CLI,
e.g. Dask and Dockerized Airflow execution.

Test Plan

Unit

Diff Detail

Repository
R1 dagster
Branch
rm-sync-exec
Lint
Lint OK
Unit
No Unit Test Coverage

Event Timeline

max created this revision.Sep 5 2019, 11:54 PM
alangenfeld requested changes to this revision.Sep 6 2019, 6:10 PM

while we may not want to remove the sync manager due to costs - I do think we should definitely move forward with the blocking mutation call change

python_modules/dagster-graphql/dagster_graphql/schema/errors.py
370

oh this is how we structured this ... hmmm maybe its too much to make the schema that different here

python_modules/dagster-graphql/dagster_graphql/schema/roots.py
196

it would be nice to update this type to not return any "result" fields

This revision now requires changes to proceed.Sep 6 2019, 6:10 PM

@sashank is going to take care of creating PlannedPipelineRun and making that the result the start mutation

max abandoned this revision.Sep 20 2019, 3:43 PM

abandoning this as we have moved to use the blocking executePlan mutation throughout