HomeElementl

Improved error message when missing config

Description

Improved error message when missing config

Summary:
This error message is displayed the same in both dagit and the CLI. I decided to render the expected format as a python dictionary (essentially JSON) instead of YAML, as I'd imagine that if you are seeing this error, it's more likely that you will be fixing the issue in python than in a yaml file.

Example 1: In the dagit playground, there is already a "scaffold missing config" button, so the fact that you're editing yaml and the error produced JSON is probably not a big deal (generally the playground flows are very nice, and don't require me to carefully read the specific error messages)

Example 2: You see this error for a sensor in dagit, you'll likely have the sensor config function defined using python dictionaries.

Example 3: You see this error while writing a test, you probably just have an execute_pipeline(my_pipeline) call that you can just copy-paste the expected config from the error into.

Of course, there are always cases where you'll be loading a config.yaml in python, and so this format won't be as ergonomic, but it still presents a huge upgrade over not showing anything at all.

github issue: https://github.com/dagster-io/dagster/issues/3991

Right now, if you pass in an empty config object and try to execute a pipeline that has some required solid config, you get a fairly unhelpful messaage "Missing required config entry "solids" at the root". This isn't great, because after this error message you still don't have a particularly fast route to fixing the issue. It doesn't tell you what solid needs config, and what config that solid needs.

When I see this error message, generally I am looking for the minimum set of config needed to run this pipeline. I don't want to keep expanding the config one error at a time (run_config={"solids":{}}, run_config={"solids":{"my_config_solid":{}}}, etc.).

Note that while this realistically does essentially copy-paste the CLI implementation of the scaffolding config,

  1. the functionality is actually different in how it treats scalar union types (CLI ignores, we choose the first of the union types)
  2. I am not the original sinner here (in terms of reimplementing this functionality). https://sourcegraph.com/github.com/dagster-io/dagster/-/blob/js_modules/dagit/packages/core/src/execute/scaffoldType.ts

It might not be a bad idea to unify this logic a bit better, but I'm not aware of if there are reasons that the CLI tool doesn't scaffold scalar union types (for example)

Test Plan: bk

Reviewers: sandyryza, alangenfeld

Reviewed By: alangenfeld

Differential Revision: https://dagster.phacility.com/D7419

Details

Provenance
owenAuthored on Apr 16 2021, 10:22 PM
Reviewer
alangenfeld
Differential Revision
D7419: Improved error message when missing config
Parents
R1:b1b34612d917: change test to customize timeout to minimize test flakes
Branches
Unknown
Tags
Unknown