Page MenuHomePhabricator

Change ConfigEditor to use root EnvironmentSchema query
ClosedPublic

Authored by prha on Mon, Sep 30, 9:27 PM.

Details

Reviewers
schrockn
Group Reviewers
Restricted Project
Commits
R1:39020b62c420: Change ConfigEditor to use root EnvironmentSchema query
Test Plan

Selected solid subgraphs, saw mapped schemas
Stashed invalid solid subsets / invalid mode into LocalStorage, saw error selectors

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

prha created this revision.Mon, Sep 30, 9:27 PM
prha added a reviewer: Restricted Project.Mon, Sep 30, 9:28 PM
prha updated this revision to Diff 5226.Mon, Sep 30, 9:41 PM

add mode error snapshot test

prha updated this revision to Diff 5227.Mon, Sep 30, 9:48 PM

type generation

schrockn requested changes to this revision.Mon, Sep 30, 10:01 PM
schrockn added a subscriber: schrockn.

req'ing changes based on js client default params stuff. seems trivial but I want to make sure to understand what is going on.

js_modules/dagit/src/__tests__/ConfigEditorModePicker.test.tsx
30

hmmmm seems like these should have default params instead of encoding at callsites

python_modules/dagster-graphql/dagster_graphql/implementation/environment_schema.py
20

would rather see this as one-liner on line 16

This revision now requires changes to proceed.Mon, Sep 30, 10:01 PM
prha updated this revision to Diff 5234.Mon, Sep 30, 10:02 PM

remove orphaned type file

schrockn requested changes to this revision.Mon, Sep 30, 10:03 PM

re-req'ing changes you might have missed

This revision now requires changes to proceed.Mon, Sep 30, 10:03 PM
prha added inline comments.Mon, Sep 30, 10:43 PM
python_modules/dagster-graphql/dagster_graphql/implementation/environment_schema.py
20

the mode "default" depends on a successful fetch of the dagster_pipeline in line 18, which might raise a user-facing graphql exception.

prha updated this revision to Diff 5248.Mon, Sep 30, 10:43 PM

address schrockn

schrockn accepted this revision.Tue, Oct 1, 1:45 AM

lgtm

This revision is now accepted and ready to land.Tue, Oct 1, 1:45 AM
schrockn added inline comments.Tue, Oct 1, 1:46 AM
python_modules/dagster-graphql/dagster_graphql/implementation/environment_schema.py
20

ah i see makes sense. thx

This revision was automatically updated to reflect the committed changes.