Page MenuHomePhabricator

Improve the error message when loading an empty workspace.yaml
ClosedPublic

Authored by max on Wed, Oct 7, 8:35 PM.

Details

Summary

Before:

dagster.check.ParameterCheckError: Param "workspace_config" is not one of ['dict', 'frozendict']. Got None which is type <class 'NoneType'>.

After:

dagster.check.ParameterCheckError: Param "workspace_config" is not one of ['dict', 'frozendict']. Got None which is type <class 'NoneType'>. Could not parse a valid workspace config from the yaml file at workspace.yaml
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 requested review of this revision.Wed, Oct 7, 9:08 PM

Is this an error that we expect a user to see?

I realized this is a user-facing error. This looks good, but I feel like we shouldn't be using check.**_param calls for things like this. The workspace_config argument is irrelevant to the user here. What do you think about doing an isinstance check (either in this function or before calling ensure_workspace_config) and throwing a DagsterInvariantViolationError or similar if we see that the config is not a dict.

This revision is now accepted and ready to land.Thu, Oct 8, 4:23 PM
python_modules/dagster/dagster/cli/workspace/config_schema.py
22–24

Maybe something like "Check that the file contains valid yaml?" at the end to give an actionable suggestion?