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.

Sign up for GitHub

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

Jump to bottom

Upgrade OpenAPI spec to v3 #989

Merged
merged 17 commits into from
Dec 12, 2022
Merged

Upgrade OpenAPI spec to v3 #989

merged 17 commits into from
Dec 12, 2022

Conversation

MattiSG
Copy link
Member

@MattiSG MattiSG commented Mar 23, 2021 •
edited by bonjourmauko
Loading

Depends on #1160

New Features

  • Upgrade OpenAPI specification of the API to v3 from Swagger v2.
  • Continuously validate OpenAPI specification.

Breaking changes

  • Drop support for OpenAPI specification v2 and prior.
  • Users relying on the aforesaid can use this to migrate (example).

screenshot

Last updated by @maukoquiroga at 2022-11-03T23:14:44Z

@MattiSG MattiSG marked this pull request as draft March 23, 2021 21:28
@MattiSG MattiSG changed the title [WIP] Upgrade OpenAPI spec to v3 Upgrade OpenAPI spec to v3 Mar 23, 2021
@bonjourmauko bonjourmauko added the kind:solution A feature request, a feature deprecation label Mar 30, 2021
@bonjourmauko
Copy link
Member

Awesome 😃 !

@openfisca/international-maintainers if this is considered a problem to have WIP PRs in this repo, please feel free to close 🙂

I think that thanks to Github drafts, WIP pull requests have been more widely and less wildly used.

I may also have some time in the coming weeks to put into this, if you need some help, haven't tried yet but I guess the validation test that you added is to help follow through programatically with the upgrade?

@MattiSG
Copy link
Member Author

MattiSG commented Mar 31, 2021

I guess the validation test that you added is to help follow through programatically with the upgrade?

It is a way to know what's left to do, yes.

@bonjourmauko bonjourmauko changed the title Upgrade OpenAPI spec to v3 [WIP] Upgrade OpenAPI spec to v3 Apr 1, 2021
@MattiSG
Copy link
Member Author

MattiSG commented Oct 7, 2022

Interesting point to have in mind: there is a user request for customising some parts of the generated OpenAPI, see openfisca/openfisca-doc#222.

@bonjourmauko bonjourmauko changed the title [WIP] Upgrade OpenAPI spec to v3 Upgrade OpenAPI spec to v3 Nov 3, 2022
@bonjourmauko bonjourmauko marked this pull request as ready for review November 3, 2022 23:15
@bonjourmauko bonjourmauko requested review from sandcha, guillett and bonjourmauko and removed request for bonjourmauko November 3, 2022 23:47
MattiSG
MattiSG commented Nov 11, 2022
View reviewed changes
Copy link
Member Author

@MattiSG MattiSG left a comment

Choose a reason for hiding this comment

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

Since I initially opened this PR, I cannot approve or request changes 😉

Here, it would be “request changes” because of the version number and the missing documentation for the null object where the conversation was resolved with no information. But otherwise everything seems good! 👍

I see you prepared openfisca/extension-template#47 and openfisca/country-template#128 for Core upgrade, but I do not see any PR in https://github.com/openfisca/legislation-explorer, while this will clearly break https://demo.openfisca.org when this upgrade gets deployed. This is not blocking for this Core upgrade however, since there should be no automated deployment without version constraint changes on Template.

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
tests/web_api/test_spec.py Outdated Show resolved Hide resolved
openfisca_web_api/openAPI.yml Outdated Show resolved Hide resolved
This was referenced Nov 14, 2022
Merged
Closed
Closed
@bonjourmauko bonjourmauko force-pushed the openapi-v3 branch 3 times, most recently from c4161d6 to 8505670 Compare December 1, 2022 17:33
@bonjourmauko
Copy link
Member

I'd like to invoke #1159 and deploy a pre-release. @guillett would you like to test it with MesAides/1jeune1solution?

@coveralls
Copy link

coveralls commented Dec 1, 2022 •
edited
Loading

Coverage Status

Coverage increased (+0.02%) to 77.659% when pulling be3dee5 on openapi-v3 into 78c0d5f on master.

@MattiSG
Copy link
Member Author

MattiSG commented Dec 6, 2022

I'd like to invoke #1159 and deploy a pre-release.

I am not sure I understand the reason: this PR is barely 600 LoC, and most of it is about moving the contents of a YAML file around. Could you please clarify what makes you consider that “the changeset is too large for manual review”? 🙂

@bonjourmauko bonjourmauko merged commit 5f95501 into master Dec 12, 2022
@bonjourmauko bonjourmauko deleted the openapi-v3 branch December 12, 2022 07:11
This was referenced May 14, 2024
Open
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bonjourmauko bonjourmauko bonjourmauko approved these changes

@sandcha sandcha Awaiting requested review from sandcha

@guillett guillett Awaiting requested review from guillett

Assignees
No one assigned
Labels
kind:solution A feature request, a feature deprecation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MattiSG @bonjourmauko @coveralls