-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
More helpful error message when schema validation fails #160
Comments
Agreed that the error message should be more descriptive. This error is being raised in feedstock.py, and taking a closer look at this, I now feel I disagree with this comment that I left there: pangeo-forge-runner/pangeo_forge_runner/feedstock.py Lines 81 to 83 in a581604
This double-validation feels like a code smell to me. The validation should just happen in the MetaYaml schema, and if that validation is leaky, we should fix it there, not double-check it somewhere else (as we are now doing, which is my fault). So the place this should be fixed is here: pangeo-forge-runner/pangeo_forge_runner/meta_yaml.py Lines 42 to 47 in a581604
Interestingly, this block did not raise an error (i.e. the validation here is currently leaky), which suggests to me that the issue is that we are allowing extra fields in that validation block, which I think we should not be doing. |
This sounds like the assert caught an issue with the schema validation actually being leaky, so it can be fixed there. Sounds like a win + positive result of the comment you left + assert there :) |
Over in leap-stc/data-management#75 i encountered a rather cryptic error:
It seemed related to the new schema validation but I had to ask @cisaacstern to get to the bottom of the problem.
Could we raise a more informative error message like:
The text was updated successfully, but these errors were encountered: