Page MenuHomePhabricator

(container-supported-dagit-2) - Integrate DagsterContainerGraphQLContext into pipeline_or_X resolvers and add preset error handling.
ClosedPublic

Authored by themissinghlink on Apr 7 2020, 5:27 PM.

Details

Summary

This revision continues to hook the DagsterContainerGraphQLContext into the dagster-graphql resolvers. Now that we are resolving individual pipelines with snapshots, we can grab presets which we want to error out on. This builds that logic in.

Depends on: https://dagster.phacility.com/D2461

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

themissinghlink edited the summary of this revision. (Show Details)Apr 7 2020, 5:31 PM
schrockn requested changes to this revision.Apr 7 2020, 11:00 PM
schrockn added inline comments.
python_modules/dagster-graphql/dagster_graphql/implementation/fetch_pipelines.py
96

instead of this, let's assert that solid_subset is None and fail if it isn't with a good error message. Explicit failure is nicer than silent wrong behavior

python_modules/dagster-graphql/dagster_graphql/schema/pipelines.py
175–179 ↗(On Diff #11511)

take care of this in previous PR and rebase

python_modules/dagster-graphql/dagster_graphql_tests/graphql/test_misc.py
291–336

I think you can make all this testing considerably simpler by making DagsterContainerGraphQLContext take the repository snapshot as an argument rather than deal with all this mocking

This revision now requires changes to proceed.Apr 7 2020, 11:00 PM
  • addressed feedback
  • rebase
Harbormaster completed remote builds in B9350: Diff 11563.
schrockn added inline comments.Apr 8 2020, 8:34 PM
python_modules/dagster-graphql/dagster_graphql/implementation/fetch_pipelines.py
78

I think it would be good to start tracking all the places we branch of isinstance(..). We will want to consolidate these once we are drive all the capabilities of DauphinPipeline from the container-based variant

schrockn accepted this revision.Apr 8 2020, 8:39 PM
schrockn added inline comments.
python_modules/dagster-graphql/dagster_graphql_tests/graphql/test_misc.py
291

nice. feels way better without the mock

This revision is now accepted and ready to land.Apr 8 2020, 8:39 PM