feat(event_handler): add custom response validation in OpenAPI utility #6189
Conversation
… error status code.
|
Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need. |
|
Hey hey @amin-farjadi! Thanks a lot for this PR! I'm working on fixing something else and will review it by Monday, ok? |
Hi @leandrodamascena 👋 Absolutely fine, thanks |
I see that our CI is failing with some importing or type annotation. Can you take a look please? |
|
Why are the tests checked against python 3.9 when the codebase is not compatible with it? Union operator for typing is used extensively in the codebase (which fails the python 3.9 test). If I turn |
There are some inconsistencies when using Pydantic models and Union operations.. Try to make this Optional and remove |
|
I see! |
|
@leandrodamascena added ruff rule |
There was a problem hiding this comment.
Hey @amin-farjadi! This is an excellent PR, and I really appreciate your work on this. I've left some comments that I believe we should address before moving to the next round of review.
I'm also curious about potentially enabling this response validation code for individual routes as well. We could have it at both the global level and per route. This isn't a mandatory change, but I'd like to hear your thoughts on this idea.
Let me know what you think, and thanks again for your valuable contribution!
aws_lambda_powertools/event_handler/middlewares/openapi_validation.py
Outdated
Show resolved
Hide resolved
|
Hi @leandrodamascena, thanks for the review. Will implement suggested changes by the end of the week. Regarding custom http status code on a route-level, I will familiarise myself with the |
Thanks @amin-farjadi! It's great that you understand the |
…lidation_error_http_code
…error_http_code param.
…r_http_code param.
…r_http_code param being None.
|
Hi @leandrodamascena, I believe all your points have been addressed in the recent commits. I have drafted an implemented for route-specific custom response validation http status code which I can add in a subsequent PR |
Hi @amin-farjadi! Thanks a lot for addressing all the points! I'll review it again today and I'll probably have some very small changes to make, especially in the documentation part, but nothing critical and I can push a commit before approving it, ok? I'm very happy to have this new feature, it will give customers more flexibility when working with data validation. Regarding to the validation code per route, please open an issue with a pseudo code showing the experience, this will be super nice! |
I'm happy to try and close this PR today. Absolutely fine by me, please do feel free. I drafted this issue for route-specific custom response validation http status code (that's a mouthful!) #6245 |
|
Hi @amin-farjadi! Just a quick explanation. A while ago you opened this issue and we fixed it in this PR. But this created a regression and I had to revert it while I find the right solution. So because of this, some tests in the
|
Hi @leandrodamascena, this has been added. Please let me know if further changes are required. |
|
There was a problem hiding this comment.
Hi @amin-farjadi! Thank you very much for working on all the feedback here! This is a super improvement for this utility!
APPROVED!
|
Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience! |
|
With pleasure @leandrodamascena 🙌 |
Issue number: #5888
Summary
Add the option to distinguish between payload validation and response validation.
Changes
has_response_validation_errorargument has been added toOpenAPIValidationMiddleware.response_validation_error_http_statusargument has been added to theApiGatewayResolverand its subclasses.ResponseValidationErrorexception class has been added to OpenAPI exceptionsAPIGatewayRestResolverresponse validation.User experience
Nothing will change. The user will have the option to set a custom response validation http status for Amazon API Gateway REST and HTTP APIs, Application Load Balancer (ALB), Lambda Function URLs, and VPC Lattice.
Checklist
If your change doesn't seem to apply, please leave them unchecked.
Is this a breaking change?
RFC issue number:
Checklist:
Acknowledgment
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.