Skip to content
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

fix(event_handler): OpenAPI schema version respects Pydantic version #3860

Merged
merged 12 commits into from Mar 1, 2024

Conversation

rubenfonseca
Copy link
Contributor

@rubenfonseca rubenfonseca commented Feb 27, 2024

Issue number: #3731

Summary

Changes

Please provide a summary of what's being changed

This PR forces the OpenAPI schema version to follow the Pydantic version. Pydantic v1 only generates OpenAPI Schema 3.0.x, and Pydantic v2 only generates OpenAPI Schema 3.1.x.

List of tasks in this PR:

  • Determines the default OpenAPI schema version according to the installed Pydantic version
  • Emits a warning if the user requests an OpenAPI schema version that is incompatible with the current installed Pydantic version
  • Adapts the schema to version 3.0.x and 3.1.x, removing things that are unnecessary
  • Add automated tests for both pydantic v1 and v2 that generate simple and complex schemas, and validate them against the official OpenAPI spec (using fastjsonschema)
  • Refactored the pytest fixtures to only load them once per session, making tests much faster

Pending items:

  • Change event handler documentation to explain when the different schema versions are generated
  • Updated Bedrock Agents documentation to explain why it will not work if you use Pydantic v2 (Agents for Amazon Bedrock seems to only support 3.0.x OpenAPI schemas) moved this checklist to the Bedrock Agents PR
  • Upgrade Swagger-UI to support 3.1 OpenAPI schemas

User experience

Please share what the user experience looks like before and after this change

After this change, the generated OpenAPI schema should always validate correctly against the official schemas

Checklist

If your change doesn't seem to apply, please leave them unchecked.

Is this a breaking change?

RFC issue number:

Checklist:

  • Migration process documented
  • Implement warnings (if it can live side by side)

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.

@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 27, 2024
@rubenfonseca rubenfonseca linked an issue Feb 27, 2024 that may be closed by this pull request
@github-actions github-actions bot added the bug Something isn't working label Feb 27, 2024
@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Feb 27, 2024
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Feb 27, 2024
@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Feb 27, 2024
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Feb 27, 2024
@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Feb 27, 2024
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Feb 27, 2024
@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Feb 27, 2024
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Feb 27, 2024
@rubenfonseca rubenfonseca marked this pull request as ready for review February 27, 2024 16:10
@rubenfonseca rubenfonseca requested a review from a team as a code owner February 27, 2024 16:10
@kbakk
Copy link
Contributor

kbakk commented Feb 29, 2024

@rubenfonseca Note that the swagger-ui version that aws-lambda-powertools ships (4.x) doesn't support OpenAPI schema version 3.1.x – you'll get an error when using it (the entire ui is inaccessible):

Please indicate a valid Swagger or OpenAPI version field. Supported version fields are swagger: "2.0" and those that match openapi: 3.0.n (for example, openapi: 3.0.0).

It does seem like passing swagger_base_url="https://cdnjs.cloudflare.com/ajax/libs/swagger-ui/5.11.8" to enable_swagger mitigates this (swagger-ui is then showing), so you might want to consider updating swagger-ui as well.

See compatibility table here: https://github.com/swagger-api/swagger-ui?tab=readme-ov-file#compatibility

@rubenfonseca
Copy link
Contributor Author

@rubenfonseca Note that the swagger-ui version that aws-lambda-powertools ships (4.x) doesn't support OpenAPI schema version 3.1.x – you'll get an error when using it (the entire ui is inaccessible):

Please indicate a valid Swagger or OpenAPI version field. Supported version fields are swagger: "2.0" and those that match openapi: 3.0.n (for example, openapi: 3.0.0).

It does seem like passing swagger_base_url="https://cdnjs.cloudflare.com/ajax/libs/swagger-ui/5.11.8" to enable_swagger mitigates this (swagger-ui is then showing), so you might want to consider updating swagger-ui as well.

See compatibility table here: swagger-api/swagger-ui?tab=readme-ov-file#compatibility

This is awesome feedback, thank you so much @kbakk !

@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Feb 29, 2024
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Feb 29, 2024
@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Feb 29, 2024
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Feb 29, 2024
@rubenfonseca
Copy link
Contributor Author

I've updated Swagger-UI to 5.x and verified that it works when using Pydantic v2 (which generated a 3.1.0 schema). @leandrodamascena this is now ready for review

@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Feb 29, 2024
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Feb 29, 2024
Copy link
Contributor

@leandrodamascena leandrodamascena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @rubenfonseca! I left some comments and we need to resolve them before I review this PR again and merge. Thanks for fixing this issue.

docs/core/event_handler/api_gateway.md Outdated Show resolved Hide resolved
docs/core/event_handler/api_gateway.md Show resolved Hide resolved
@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Mar 1, 2024
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Mar 1, 2024
@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Mar 1, 2024
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Mar 1, 2024
@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Mar 1, 2024
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Mar 1, 2024
@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Mar 1, 2024
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Mar 1, 2024
@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Mar 1, 2024
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Mar 1, 2024
Copy link

sonarcloud bot commented Mar 1, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link
Contributor

@leandrodamascena leandrodamascena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVED!

@sthulb sthulb merged commit 579662e into develop Mar 1, 2024
14 checks passed
@sthulb sthulb deleted the rf/3731 branch March 1, 2024 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working event_handlers size/L Denotes a PR that changes 100-499 lines, ignoring generated files. tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: OpenAPI schema invalid when using Pydantic v2 (latest)
4 participants