Page MenuHomePhabricator

Use executePlan within dagster-graphql-mediated execution engines
ClosedPublic

Authored by max on Tue, Sep 10, 10:23 PM.

Details

Summary

This is to avoid creating spurious runs in run storage.

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.Tue, Sep 10, 10:23 PM
max retitled this revision from WIP to Use executePlan within dagster-graphql-mediated execution engines.Tue, Sep 10, 10:54 PM
max edited the summary of this revision. (Show Details)
max added reviewers: alangenfeld, Restricted Project.
schrockn accepted this revision.Wed, Sep 11, 2:12 AM
schrockn added a subscriber: schrockn.

this is great.

Please add some manual testing within dagit to your test plan as this is the pretty significant change. It's a bit tedious but I think exercising the pipelines in our examples repo would be appropriate here.

python_modules/dagster-graphql/dagster_graphql/client/mutations.py
107–146

This is a pre-existing issue, but it seems like this type of processing is pretty unnecessary and we should just have a generic codepath for all error types. Meaning that in the error case we could pass along a message, a type, and the graphql payload, and there would be no required specialization per-type at this layer.

This revision is now accepted and ready to land.Wed, Sep 11, 2:12 AM