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

Bump OAPI 2 to OAPI3 (Autogenerated) #143

Merged
merged 1 commit into from
Sep 21, 2020
Merged

Bump OAPI 2 to OAPI3 (Autogenerated) #143

merged 1 commit into from
Sep 21, 2020

Conversation

susheel
Copy link
Member

@susheel susheel commented Jan 28, 2020

Will need to check and verify before merging as the changes are extensive.

This is a dependency to set up the service-info PR #144

Will need to check and verify before merging as the changes are extensive
@susheel susheel changed the title OA3 upgrade (Autogenerated) Bump OAPI 2 to OAPI3 (Autogenerated) Jan 28, 2020
@dglazer
Copy link
Member

dglazer commented Jan 28, 2020

@susheel , the last time I remember us talking about upgrading to OAPI3, we didn't know how to get the auto-generated documentation to work, so we decided to stick with OAPI2. Has that been resolved? (I'm guessing not, since the Travis CI checks failed, but maybe those are unrelated errors?). Adding @jaeddy in case he remembers more about the details.

@susheel susheel changed the base branch from master to release/v1.0.1 January 28, 2020 14:05
@briandoconnor
Copy link
Contributor

briandoconnor commented Jan 30, 2020

Yep, I see this error in the travis CI build log:

The command "pip install . --process-dependency-links" failed and exited with 2 during .

@susheel I'm assuming you bumped the version to 3 so you could use this construct to extend service-info:

"Your OAS 3 API might want to define additional service information as well. To do that, use the allOf OAS 3 construct."

Is that right?

@susheel
Copy link
Member Author

susheel commented Jan 30, 2020

@briandoconnor Yes, we needed to bump it to OAPI3 to support the allOf construct. I haven't been able to check the travis build, but the autogenerated YAML works in the online swagger checker for the OA3-upgrade yaml.

We may need to bump the travis build packages to OAPI3 too.

Copy link
Member

@dglazer dglazer left a comment

Choose a reason for hiding this comment

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

@susheel , I understand that OAPI 3 supports some new features. However, at least last time we looked at it, it also broke the auto-generated documentation, so we decided not to take advantage of those new features. Not a great choice to have to make, but until it's resolved, we either have to find a whole new way of keeping our documentation up to date, or stick with OAPI 2. @jaeddy may remember more details.

However we resolve, we need to make sure we'll have clean updated documentation before merging this in.

@susheel susheel changed the base branch from release/v1.0.1 to develop January 30, 2020 15:20
@jaeddy
Copy link
Member

jaeddy commented Feb 9, 2020

Hey @susheel, @dglazer — I've been working on a little tool for the past few months that would replace swagger2markup (no support for OpenAPI 3.0) with ReDoc and/or Sphinx+ReadTheDocs. This would also address some of our build issues (and vulnerability warnings) by moving that logic to an independent package.

It's been a while since I've been able to make progress on the tool, but I'll discuss with another engineer on my team and see if he can pick up the remaining development to get something working. Once we get those changes made, it'd be great to review and merge this PR.

@jaeddy jaeddy mentioned this pull request Jun 22, 2020
@jaeddy jaeddy merged commit 4bb5dae into develop Sep 21, 2020
@jaeddy jaeddy deleted the OA3-upgrade branch September 21, 2020 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants