-
Notifications
You must be signed in to change notification settings - Fork 407
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
feat(openapi): enhance support for tuple return type validation #5997
feat(openapi): enhance support for tuple return type validation #5997
Conversation
Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need. |
f6f7034
to
6a62cb1
Compare
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.
Hey @philiptzou! Thanks a lot for submitting this PR. 🚀
I see we have some problems with type checking that is breaking the CI and we need to address them before I make a final review.
tests/functional/event_handler/_pydantic/test_openapi_params.py
Outdated
Show resolved
Hide resolved
tests/functional/event_handler/_pydantic/test_openapi_params.py
Outdated
Show resolved
Hide resolved
6a62cb1
to
536f4cd
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #5997 +/- ##
========================================
Coverage 96.24% 96.24%
========================================
Files 234 234
Lines 11017 11024 +7
Branches 798 800 +2
========================================
+ Hits 10603 10610 +7
Misses 327 327
Partials 87 87 ☔ View full report in Codecov by Sentry. |
Quality Gate passedIssues Measures |
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.
Hey @philiptzou! Thanks for working on this PR and addressing the feedback!
I have a feeling that this PR can breaks the validation for people using dict as return. But I think this is really a bug that we need to fix.
e.g
@app.get('/foobar')
def foobar() -> dict:
return Result(payload='foobar'), 201
APPROVED!
Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience! |
Well, I don't think this PR will break dict as return since it still falls in the |
Issue number: #5034
Summary
Changes
This PR fixes the issue that OpenAPI validation failed to validate two-element tuple returns, which is allowed by the
APIGateway
.User experience
Below test fails before this PR but passes after this PR:
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.