Add ability to check openapi responses #36
Conversation
|
Please squash commits together. |
Done. |
| for missing_response in missing_responses: | ||
| check_failed = True | ||
| missing_responses_count += 1 | ||
| errors.append( |
There was a problem hiding this comment.
This feels like it should be Error object with repr and str dunders.
mandarvaze
force-pushed
the
master
branch
2 times, most recently
from
22728b4
to
477dbd0
Compare
zupo
changed the title
This can be used to ensure all API endpoints support common responses. Path to responses.yaml needs to be provied via config `pyramid_openapi3_responses_config`
|
Hi @mandarvaze I've been taking a look at this, after reading issue #22. I think this is a really good base, but I think there's a chicken-and-egg situation here. If we require users to supply a validation document in order to get any warnings, then it doesn't really help users who don't know what they're doing. I think it would be really valuable if we could include a default spec, and users can override that if they want to have non-compliant responses. I also think that raising an exception might be a bit heavy-handed, especially the ResponseValidation exception. What would you think about raising a warning for each API endpoint that's noncompliant? |
|
@MatthewWilkes Unfortunately I have lost context after 7 months :( But @zupo and @dz0ny may have ideas related to your queries. |
|
@mandarvaze Totally understandable. Would you object if I made some tweaks with this PR as a base? |
|
@MatthewWilkes Please go ahead 👍 |
|
Closing due to inactivity. That said, I still see value in this PR and plan to circle back to it when I have some time on my hand. |
A common pitfall when using this package is the following: you define an endpoint that can result in 400 Bad Request, but you forget to list 400 in the `responses` section of your endpoint in openapi.yaml. This package then instead returns 500 Internal Server error, because it keeps the promise that only defined responses will be allowed (unless you set `enable_request_validation` to `False`, that is). With this commit, all endpoints, by default need to have 200, 400 and 500 on the list of `responses` in openapi.yaml, otherwise the app won't start. Additionally, all endpoints that accept a parameter, also need to have 404 on the list of `responses`. You can skip this check by setting `enable_endpoint_validation` to `False`. Refs #22 Refs #49 (comment) Refs #36
A common pitfall when using this package is the following: you define an endpoint that can result in 400 Bad Request, but you forget to list 400 in the `responses` section of your endpoint in openapi.yaml. This package then instead returns 500 Internal Server error, because it keeps the promise that only defined responses will be allowed (unless you set `enable_request_validation` to `False`, that is). With this commit, all endpoints, by default need to have 200, 400 and 500 on the list of `responses` in openapi.yaml, otherwise the app won't start. Additionally, all endpoints that accept a parameter, also need to have 404 on the list of `responses`. You can skip this check by setting `enable_endpoint_validation` to `False`. Refs #22 Refs #49 (comment) Refs #36
This can be used to ensure all API endpoints support common responses.
Path to responses.yaml needs to be provied via config
pyramid_openapi3_responses_config