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

Support for OpenAPI >= 3.0.0 #265

Closed
fox1t opened this issue Jul 9, 2020 · 28 comments · Fixed by #333
Closed

Support for OpenAPI >= 3.0.0 #265

fox1t opened this issue Jul 9, 2020 · 28 comments · Fixed by #333

Comments

@fox1t
Copy link
Member

fox1t commented Jul 9, 2020

🚀 Feature Proposal

I was wondering if there is any reason why we are using only v2 of Swagger spec. Therefore, I propose to add an option during the plugin registration to allow the users to choose between v2 and v3 when dynamic mode is used.

Motivation

They are some useful features of OpenAPI v3 that are used by https://github.com/sinclairzx81/typebox to generate the awesome typed schemas that are used by Fastify routes.

@delvedor
Copy link
Member

delvedor commented Jul 9, 2020

I would love to see this!
We are currently using swagger because it was the first implementation and no one wanted to migrate to open api.
If you have time, feel free to! :)

@fox1t
Copy link
Member Author

fox1t commented Jul 10, 2020

@delvedor would be fine to use something like this internally?

@delvedor
Copy link
Member

@fox1t Unfortunately I don’t have enough knowledge about open api vs swagger.
Why a tool like that would be needed?

cc @mcollina

@fox1t
Copy link
Member Author

fox1t commented Jul 10, 2020

V2 differs from v3 because the general structure has changed (params, body and so on) and some keywords where added (such as oneOf). I was wandering if it is enough to generate always the v2 (as this package already does) and then convert it to v3 automatically if the option has been passed.

@delvedor
Copy link
Member

If the industry is consistently moving to open api, I would migrate the package to open api entirely :)

@fox1t
Copy link
Member Author

fox1t commented Jul 11, 2020

Actually at the moment we are in the middle of Python 2/3 problem here: swagger 2.0 is still largely used and OpenAPI 3.0.0 is emerging as new standard. I am not totally confident to drop support for 2.0 yet.

@mcollina
Copy link
Member

Having an config parameter would be good.

@Eomm
Copy link
Member

Eomm commented Jul 12, 2020

@delvedor would be fine to use something like this internally?

In this case, we should:

  • expose OAS3 feature
  • generate swagger (OAS2)
  • convert to OAS3
  • merge the two objects

I think it is not much flexible.

Right now the code load the static.js or dynamic.js file to generate the API console, let's say.
I would just load a dynamic_oas3.js to implement this format composing (there will be for sure replicated code at first version - but it will less painful for bugs or new oas3 features)


JSONSchema is going to be compatible with OAS3, I hope that when it will exist from draft status, it will be 100% so the effort to implement this will be near 0 🔝

@harveyconnor
Copy link

Any progress on this? We're looking at integrating fastify-swagger with our application however we rely on OAS3. And the other fastify-oas library does not meet our spec...

@kibertoad
Copy link
Member

kibertoad commented Aug 17, 2020

I could pick this up (OpenAPI 3 def is a huge thing now) if someone could point me out in the direction of what needs to be done.

@harveyconnor
Copy link

@kibertoad That would be awesome! @Eomm may be able to point you in the right direction?

@kibertoad
Copy link
Member

However, swagger is very much specifically OpenAPI 2. So it might make sense to fork the plugin into fastify-openapi

@fox1t
Copy link
Member Author

fox1t commented Aug 17, 2020

However, swagger is very much specifically OpenAPI 2. So it might make sense to fork the plugin into fastify-openapi

True. But it would be nice to have just one package that does both

@kibertoad
Copy link
Member

@fox1t Then it would make sense to call that fastify-openapi and deprecate swagger one.

@fox1t
Copy link
Member Author

fox1t commented Aug 17, 2020

Swagger is still very used and it is better to continue to support it. As I already said it is a Python 2/3 problem. :)
Another thing to consider is the fact that most of the code, descriptions, and so on are shared between the 2 versions. The only real difference is the produced JSON. That's why I think it is better to have it in one place and let the user decide, using an option if she/he wants v2 or v3.

@harveyconnor
Copy link

I think the best way is to maintain two separate branches in here.
oas v3 & oas v2.
Love to see this progress as OAS 3 is vital to our applications.

@DRoet
Copy link

DRoet commented Aug 26, 2020

I think it might be worth noting here that there is also fastify-oas that supports openapi 3.0 and now works with fastify v3 aswell.

@fox1t
Copy link
Member Author

fox1t commented Aug 26, 2020

I am going to say this again, for future reference: I still think that having one plugin support them both is the best choice.

@harveyconnor
Copy link

Agreed, a single repository will allow for a more community engagement.

@mcollina
Copy link
Member

Agreed, a single repository will allow for a more community engagement.

There is literally nothing blocking that minus working for supporting both swagger and openapi on the same module.

fastify-oas exists because of #87. The issue is literally still there.

@benediktdertinger
Copy link
Contributor

There is literally nothing blocking that minus working for supporting both swagger and openapi on the same module.

fastify-oas exists because of #87. The issue is literally still there.

Hi guys, how can we support you integrating both standards into one repo?

@fox1t
Copy link
Member Author

fox1t commented Sep 8, 2020

@benediktdertinger, as @mcollina pointed out, we should start with #87

@benediktdertinger
Copy link
Contributor

@fox1t I already checked #87 but I'm not sure what it actually means for offering support as you guys wrote you'd prefer one instead of two repos, right? I already switched to fastify-oas myself and it is working quite well. Should we place issues and PRs in the fastify-oas repo in the future?

@fox1t
Copy link
Member Author

fox1t commented Sep 8, 2020

Ah ok.
What we mean here is that it would be better to have a new option to pass to fastify-swagger during the registration time of the plugin that selects which version of OpenAPI/Swagger the user wants to use. This would be better instead of having 2 separate packages.

@benediktdertinger
Copy link
Contributor

@fox1t thanks for clarifying. Makes sense to me. It is still unclear for me wether I should focus on the fastify-oas package regarding changes/issues/PRs or stay with your package fastify-swagger?

@fox1t
Copy link
Member Author

fox1t commented Sep 14, 2020

@benediktdertinger the idea is to add functionalities to fastify-swagger!

@carlosfaria94
Copy link

JSONSchema is going to be compatible with OAS3, I hope that when it will exist from draft status, it will be 100% so the effort to implement this will be near 0 🔝

JSON Schema for the OpenAPI 3.0 Specification have beed released OAI/OpenAPI-Specification#1032

https://github.com/OAI/OpenAPI-Specification/tree/master/schemas/v3.0

@climba03003
Copy link
Member

I think this plugin should move on to OAS3 and convert it to Swagger 2.0 for backward compatible.
The main reason is that, fastify support OAS3 syntax like oneOf, anyOf and 2XX response. However, Swagger 2.0 lack of those syntax support. It always break the swagger interface or missing some of the fields.

I am willing to make an PR for the OAS3 feature and I want the dev team to confirm the right way to implement this.

Resources that maybe useful in below
OAS3 and Swagger 2.0 difference
Swagger Parser
API Spec Converter

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 a pull request may close this issue.

10 participants