-
-
Notifications
You must be signed in to change notification settings - Fork 68
On validation error: add cause and context. #129
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
Conversation
Oh I just remembered jsonschema validator had a best_match function! I'm using it now so it's less floody / more readable:
|
I'm not happy with the Any idea? |
This made error messages slightly more useful when trying to deal with upgrading past 0.3.0 and running into some minor adaptive changes required. |
Maybe |
Updated my PR with this, it would work for me. |
@JulienPalard thanks looks good, can you check failing test? |
I'm having a look at the failing test. |
I added a test and tried to make the message a bit readable (having all the subschema dumped in a serie was a bit hard to digest). It currently looks like:
I find the "markdown-y" |
Codecov Report
@@ Coverage Diff @@
## master #129 +/- ##
==========================================
- Coverage 98.46% 98.14% -0.33%
==========================================
Files 19 19
Lines 520 538 +18
==========================================
+ Hits 512 528 +16
- Misses 8 10 +2
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@JulienPalard thank you for the contribution |
I often miss the context provided by jsonschema library, so I'm trying to add it.
Given the following erroneous schema (it misses a
description
):without this PR, openapi-spec-validator displays:
Basically telling « The
path./.get.responses.200
is invalid: it should be a Response or a Reference. » without telling me about the missingdescription
.With this PR it gives:
This is probably a bit verbose, but I like a lot seeing
'description' is a required property
.