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

Upgrade openapi after subscriptions removal #1821

Merged
merged 17 commits into from
Jan 10, 2024

Conversation

jtherrmann
Copy link
Contributor

@jtherrmann jtherrmann commented Sep 6, 2023

See #1193

After removing the Subscriptions feature, we've significantly changed our API spec and have removed the need for WKT string validation, which makes much of the work done on #1708 obsolete, so I'm starting over from scratch with this PR.

This PR includes the following changes:

  • Upgrade the openapi-core, openapi-spec-validator, and jsonschema packages to their latest versions
  • Minor updates to the reference paths in our OpenAPI spec files
  • Convert our API endpoints from Flask class-based views to functions decorated with @app.route, which leads to simpler code (in my opinion) and, more importantly, allows us to more easily use the FlaskOpenAPIViewDecorator decorator provided by openapi-core, which in turn allows us to re-implement the following features according to the latest openapi-core interface:
    • Disabling of response validation
    • Custom error formatting

TODO:

  • Create an issue for converting the openapi-core pin back to the normal format following the next PyPI release (see the TODO comment in the requirements file)

@jtherrmann
Copy link
Contributor Author

The cfn-lint package pins jsonschema here (see aws-cloudformation/cfn-lint#2792).

@jtherrmann
Copy link
Contributor Author

The version conflict should be resolved by the most recent version of cfn-lint.

@jtherrmann
Copy link
Contributor Author

We can now upgrade the openapi-* and jsonschema packages to their latest versions.

@jtherrmann jtherrmann marked this pull request as ready for review January 5, 2024 03:46
@jtherrmann jtherrmann requested a review from a team January 5, 2024 03:46
Copy link
Contributor

@forrestfwilliams forrestfwilliams left a comment

Choose a reason for hiding this comment

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

From what I can tell, this PR look good. I'm confident in the changes to our configuration YAMLs, the dependabot changes, the changelog, and requirements files.

However, I have not worked with Flask APIs in the past, and thus don't feel good rubber stamping the changes in apps/api/src/hyp3_api/routes.py. Would @jhkennedy or @asjohnston-asf be willing to lend a hand here?

apps/api/src/hyp3_api/routes.py Show resolved Hide resolved
Copy link
Contributor

@jhkennedy jhkennedy left a comment

Choose a reason for hiding this comment

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

Looks great to me! I like the class to decorator change.

@jtherrmann jtherrmann merged commit 557a21c into develop Jan 10, 2024
12 checks passed
@jtherrmann jtherrmann deleted the upgrade-openapi-after-subscriptions-removal branch January 10, 2024 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants