-
Notifications
You must be signed in to change notification settings - Fork 402
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(event_handler): generate OpenAPI specifications and validate input/output #3109
Conversation
03389da
to
72375cc
Compare
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## develop #3109 +/- ##
===========================================
- Coverage 95.94% 95.09% -0.85%
===========================================
Files 195 202 +7
Lines 8387 9407 +1020
Branches 1563 1719 +156
===========================================
+ Hits 8047 8946 +899
- Misses 277 341 +64
- Partials 63 120 +57
☔ View full report in Codecov by Sentry. |
|
|
|
Is this also going to have support for the Application Load Balancer event handler? |
Absolutely! All event handlers will work out of the box :) |
Hi @rubenfonseca, pretty interested by this, would it be possible to get a preview of the UX for a @app.post with a body ? :) |
Almost there, Ruben has just nailed this part but pushing changes this week to the PR. It was a difficult call to make UX wise while following our Progressive tenet and no backwards incompatible change. You'll be pleased, we hope!! |
I see that we will have the ability to spit out the JSON, but will it also be able to generate the Swagger UI like FastAPI does? Or will we have to host the Swagger UI static assets on S3 or something? |
It will auto-generate like FastAPI, including data validation. We're aiming for a FastAPI like experience. We're also considering a method to generate a fragment of the JSON Schema to allow customers with micro functions to get to the same end result with Swagger UI -- this will need a CLI of sorts, so we will need a RFC for that CLI/alternative solution later (the method is already implemented AFAIK). Ruben is deeply heads down on handling the final bits of backwards compatibility but the whole experience to enable ala FastAPI will be simply a flag + bring Pydantic as a dep. |
1fcff5c
to
b71f262
Compare
b71f262
to
d6f3264
Compare
|
@leandrodamascena had some trouble with the shared types since it broke some older versions of Python, but finally the tests are green again |
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.
Hello @rubenfonseca! I left some comments and I'm missing some tests with optional fields in Pydantic and Dataclasses. Just to make sure we are respecting the API contract and if we parser them as not required
.
That's it for me and I'm fine! I'm sure @heitorlessa wants to review it before merging.
Thanks for all the effort here, man.
Quick peer review feedback with Leandro... Split Data Validation and Serialization from OpenAPI Generation example It is not clear from the PR Body what the experience looks like BEFORE and AFTER, and what the richer experience brings to the table besides OpenAPI Schema. For example, we can now return a DTO and we will serialize it for them... we couldn't before.. but there's no JSON output here. If you split it, it becomes clearer, and we can even use this in the documentation.
Increase coverage to at least 90% JSON Encoder for example is missing several tests |
@leandrodamascena I've elevated the coverage now, and I think it's ready for another review |
Signed-off-by: Leandro Damascena <[email protected]>
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
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.
APPROVED!!!
Issue number: #2421
Summary
Changes
This PR adds support for generating OpenAPI specifications from an event handler resolver, by inspecting the handler parameters and return types. Additionally, it adds a new middleware that will validate the input and output before/after the handler is invocated, allowing one to use typed parameters in the handler signature.
User experience
Creating simple openapi spec - Python code
Creating simple openapi spec - output
Using simple get and post route - Python code
Using simple get and post route - OpenAPI spec
Using get and post route with dataclass - Python code
Using get and post route with dataclass - OpenAPI Schema
Serializing DTO - Python code
Serializing DTO - Output
Checklist
If your change doesn't seem to apply, please leave them unchecked.
TODO
The following are high level tasks necessary to finish the PR:
NEXT PR
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.