Page MenuHomePhabricator

#2261 better Execution Plan error handling
ClosedPublic

Authored by yuhan on Fri, Mar 13, 6:42 AM.

Details

Summary

Issue: https://github.com/dagster-io/dagster/issues/2261

"Playground - Execution Plan" section should surface corresponding/detailed error message instead of PythonError

Test Plan
  • unit test
  • dagit

before:


after:

  • BK

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

yuhan created this revision.Fri, Mar 13, 6:42 AM
yuhan added inline comments.Fri, Mar 13, 6:46 AM
examples/dagster_examples/intro_tutorial/serialization_strategy.py
33–36 ↗(On Diff #10482)

Is this the right way to define input_hydration_config for pickle? -- particularly for the last screenshot in https://dagster.readthedocs.io/en/latest/sections/tutorial/intermediates.html

yuhan edited the test plan for this revision. (Show Details)Fri, Mar 13, 6:47 AM
yuhan edited the test plan for this revision. (Show Details)
yuhan updated this revision to Diff 10484.Fri, Mar 13, 7:14 AM
yuhan edited the test plan for this revision. (Show Details)

update

Two fixes here

would be nice to split this up in to two diffs, I know the bug fix stuff is solid but dont know the tutorial stuff

python_modules/dagster-graphql/dagster_graphql/implementation/fetch_runs.py
19–20

this check is too late if dauphine_pipeline is bad - we want to check on dauhpin_pipeline

check.inst_param(dauphin_pipeline, 'dauphin_pipeline', graphene_info.schema.type_named('Pipeline'))
84–85

this should get updated too while we are here

python_modules/dagster-graphql/dagster_graphql_tests/graphql/test_subset.py
68

probably want to test for something about input hydration config in the message

yuhan updated this revision to Diff 10512.Fri, Mar 13, 9:31 PM

split this diff into two diffs. this current one is for dagster-graphql

yuhan marked an inline comment as done.Fri, Mar 13, 9:33 PM
yuhan added inline comments.
python_modules/dagster-graphql/dagster_graphql/implementation/fetch_runs.py
84–85

isn't this already get_dauphin_pipeline_from_selector_or_raise?

alangenfeld accepted this revision.Fri, Mar 13, 9:40 PM

cool - update title and summary before landing

redfordnod

python_modules/dagster-graphql/dagster_graphql/implementation/fetch_runs.py
84–85

oops 😎

This revision is now accepted and ready to land.Fri, Mar 13, 9:40 PM
yuhan retitled this revision from #2261 fix intermediates re-execution tutorial and better Execution Plan error handling to #2261 better Execution Plan error handling.Fri, Mar 13, 10:05 PM
yuhan edited the summary of this revision. (Show Details)
This revision was automatically updated to reflect the committed changes.