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

Add an OpenAPI 3 Serializer #468

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Conversation

kylef
Copy link
Member

@kylef kylef commented May 20, 2020

Initial proof of concept with numerous caveats and unsupported elements (tracked in STATUS.md).

  • Add smoke test for fury-cli

@kylef kylef added the openapi3 label May 20, 2020
Copy link
Contributor

@opichals opichals left a comment

Choose a reason for hiding this comment

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

Added just a note about the async interface. I guess we do callbacks for consistency which is a good thing so no need to change.

Perhaps we introduce synchronous API for everything that could be done synchronously later on.

fury.use(openapi3Serializer);

// Assume `api` is a Minim element instance, e.g. from `fury.parse(...)`
fury.serialize({ api, mediaType: 'application/vnd.oai.openapi' }, (error, content) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd expect the serialize would be synchronous for easy of use.

Copy link
Member Author

Choose a reason for hiding this comment

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

The parse and serialize functions in Fury are async. As we provide a uniform interface for serializing regardless of format and adapter implementation. This includes the "remote" adapter which uses HTTP to parse/serialize this could be problematic to support them all as sync.

I don't have much strong opinions against or for this, so if you would like sync API we can create an issue in the repository to investigate support across all the adapters.

Copy link
Member Author

Choose a reason for hiding this comment

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

FWIW, master has sync interface for serialize now. We can support it here 😄.

@BenFradet
Copy link

Hey 👋 , might I ask what's the status of this PR? Thank you 👍

@kylef
Copy link
Member Author

kylef commented Feb 11, 2021

Hey wave , might I ask what's the status of this PR? Thank you +1

This is more of a proof of concept, I'm not sure we'll pursue this any further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants