Own the swagger definition for the Service Broker API

avade commented 7 years ago

Proposal

I propose that the OSABPI WG own the [Swagger](http://swagger.io/} definition for the service broker API that is currently in the Cloud Foundry Incubator project.

I have raised this idea with the project lead Dr Max and he thought this was a good idea™

Why Swagger?

Swagger makes it possible for API providers to create interactive API documentation where developers can not only learn specifics about an API, but also experiment with, test and debug API calls. Swagger can be implemented in many different ways including Scala, Java, HTML5, JavaScript, Ruby and PHP. The Swagger core is available for download at GitHub.

Automated API documentation that is consistent, easy to read and provides the ability to make live API calls is beneficial to both API providers and API consumers. Thorough, interactive API documentation helps developers better understand and easily consume APIs and also aids in API discovery. For API providers, offering APIs that are easy to understand and consume helps in the promotion of their APIs and can increase the adoption and consumption of their APIs by developers.

https://www.programmableweb.com/news/swagger-interactive-api-documentation-framework-benefits-both-api-providers-and-consumers/2013/02/19

Questions

Would we maintain the current spec markdown or move to swagger generated markdown?

angarg12 commented 7 years ago

What would happen if we don't transfer it? Would it be kept and maintained by CF, or dropped?

Seems reasonable to me that we own the Swagger definition if we own the spec itself.

avade commented 7 years ago

What would happen if we don't transfer it? Would it be kept and maintain by CF, or dropped?

I would imagine that the CF team would continue to maintain it.

pmorie commented 7 years ago

I would support a swagger spec for this API, but it is important to preserve the context and textual description present in the current spec.md. I think of swagger/openapi as replacing the markdown tables in the current spec documents.

angarg12 commented 7 years ago

To follow on @pmorie comment, I would like to avoid duplicate information to reduce overhead and the danger of divergence. Would it be an option to move the API definition to Swagger and keep md for the textual spec definition?

Also, if we give it a go, how would the transition look like? We will need to rebase it to the latest API version, I can take that task myself.

mattmcneeney commented 7 years ago

We could link from the swagger spec to a contextual document using the External Documentation Object (from here).

What would be the next step for this? Update the latest swagger spec and create a branch of spec.md containing only the contextual information?

avade commented 7 years ago

Here is my four-step plan:

1) Create a PR to the osbapi project that shows the new files we want to add. 1) Then we go for a round of looks goods. 1) Merge the PR 1) PR to the cf-swagger project, they have asked for a pointer in their project to the new location.

duglin commented 7 years ago

sounds good to me

mattmcneeney commented 7 years ago

I don't find the standalone swagger YAMLs that easy to read, especially since they use lots of includes / link to definitions. I also haven't found a simple tool we could use to generate a Markdown spec upon changes (other than building and running the swagger editor which takes some time). Swagger has a hosted version (Swagger Hub) but can only sync from the hosted editor to Github, not the other way around.

Has anyone come across any simple swagger to markdown converters?

mattmcneeney commented 7 years ago

Following my comment yesterday, I've created a branch that links to a nicely rendered version of the YAML spec.

Just head to the branch and click on The Open Service Broker API specification.

What does everyone think of this? Hopefully this will give people new to the project a nice way of quickly understanding the spec? We can use External Documentation Objects to link away from the hosted spec whenever more context is needed.

Edit: I know this spec is a bit outdated, but I didn't want to do the work to update it unless people are happy with this solution.

angarg12 commented 7 years ago

Looks great.

Then we would proceed to replace the Markdown tables with Swagger? Meaning, removing them completely from the md to avoid duplication.

mattmcneeney commented 7 years ago

I think that's sensible (maintaining it in two places would obviously be painful). We'd keep the contextual information in spec.md and link to it wherever is needed.

mattmcneeney commented 7 years ago

I managed to include the contextual definition in the swagger file along with the spec. I've updated the branch and you can view the full spec here. Hopefully we can discuss this on this weeks call. The sooner we decide whether or not we like this, the less painful all of the merging of PRs/branches will be.

angarg12 commented 7 years ago

So then the whole spec becomes Swagger? I though the intent was to have 2 files, one Markdown with the spec, and one Swagger with the API (endpoints, parameters).

mattmcneeney commented 7 years ago

That was the original intent, but then I realised you could embed the markdown in the swagger document and thought one file would be easier to maintain than two. This is up for discussion though! On Tue, 11 Apr 2017 at 07:25, Andrés notifications@github.com wrote:

So then the whole spec becomes Swagger? I though the intent was to have 2 files, one Markdown with the spec, and one Swagger with the API (endpoints, parameters).

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/openservicebrokerapi/servicebroker/issues/160#issuecomment-293163114, or mute the thread https://github.com/notifications/unsubscribe-auth/AG7UzPpSD7mca8_9HpAYY4nDlgFQLfZbks5ruxy9gaJpZM4Myuq3 .

duglin commented 7 years ago

@mattmcneeney something seems to be missing from http://petstore.swagger.io/?url=https://raw.githubusercontent.com/mattmcneeney/servicebroker/swagger/spec.yaml - for example, the entire provisioning section and querying the catalog appears to be missing. Even the TOC is gone. Or is this just a sampling of what you're thinking of?

mattmcneeney commented 7 years ago

@duglin I started copying over the contextual info, but decided to pause working on it since this was still up for debate. Happy to copy across the remaining info and tidy it up if the group is happy to use this.

duglin commented 7 years ago

Nope, I'd wait - wouldn't want you to do work that's tossed out if the group didn't want to go that way. Just wanted to make sure it was intentional.

duglin commented 7 years ago

If we head this way, does the fact that the spec is in yaml impact anything? For example, line wrapping is changed, and that could impact the UX experience from a spec dev perspective. What about the CF build process? They seem to have some setup requirements that might not like yaml files.

Am I correct that from an end-user perspective the spec would look pretty much the same as today but with a new section at the end that shows the HTTP flows in a more technical way? If so, I'm inclined to keep them separate just because the UX of dealing with yaml instead of md will probably annoy me - I'm already annoyed we don't wrap things are 80 columns :-)

mattmcneeney commented 7 years ago

@duglin yep that's right, the only benefit could be linking to specific sections in the text part of the spec from the HTTP flow section (rather than having to jump between two different files). Personally I'd prefer to have everything in one file, and I think the hosted swagger UI does a reasonably good job at displaying the text and flow spec, but happy to go with whatever the group decide works best for the community.

wryun commented 7 years ago

Is this something that's going to happen in the next few months? The current cloudfoundry incubator definitions are now out of date with 2.12.

Happy to contribute on @mattmcneeney 's branch or help push this in whatever direction has been decided.

dizz commented 7 years ago

If it's of interest to folks there's a v2.12 swagger def of the API spec here. The @mattmcneeney version was reviewed in this context and was found to be out of sync with the then v2.11 spec. Do note there are extensions to the spec that are under the /v2/et path namespace.

kenjones-cisco commented 7 years ago

@mattmcneeney FYI, there is a Swagger2Markup option available to help with UX aspect. It is capable of converting to either Markdown or Asciidoc.

This can easily be done via a script. I typically use Docker for these to avoid having to install everything locally.

Example:

Dockerfile

FROM openjdk:8-jdk

ENV SWAGGER2MARKUP_VERSION 1.3.1

RUN mkdir /usr/local/swagger2markup \
    && BASE_DL_URL="https://jcenter.bintray.com/io/github/swagger2markup/swagger2markup-cli" \
    && wget -O /usr/local/swagger2markup/swagger2markup-cli.jar "$BASE_DL_URL/$SWAGGER2MARKUP_VERSION/swagger2markup-cli-$SWAGGER2MARKUP_VERSION.jar"

Example Command run with the container or locally:

java -jar /usr/local/swagger2markup/swagger2markup-cli.jar convert -c config.properties -i spec.yaml -d docs/gen

Example configuration file used to control the output: config.properties

swagger2markup.markupLanguage=MARKDOWN
swagger2markup.basePathPrefixEnabled=true

http://swagger2markup.github.io/swagger2markup/1.3.1/#_swagger2markup_properties

As such the single source that is manually maintained would be spec.yaml based on Swagger/OpenAPI. And then using a script, a Markdown / Asciidoc file can be generated from the spec.yaml file.

ringods commented 7 years ago

Swagger (as a specification) has evolved to the OpenAPI Specification. Please look at that towards the future: https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md

arschles commented 7 years ago

SGTM as well. I have a few comments on the spec as I've seen here: https://github.com/mattmcneeney/servicebroker/blob/swagger/spec.yaml

IMO the markdown shouldn't be copied into the info.description field of the spec. A short description and a link to the official spec should be good enough
We shouldn't consider the yaml as human readable, nor should it contain all of the details as are in the markdown version of the spec. They should live together.
- The swagger spec should be used primarily to generate client & server code. The developers will need to read the markdown spec before implementing a broker server or client
We shouldn't make it a goal to generate the markdown/detailed spec from the swagger document

rhodie27 commented 7 years ago

Agree with @arschles 100%. Swagger specs are a good way to increase adoption via server generation.

duglin commented 7 years ago

From 11/17 call, @vaikas-google will write-up a proposal for this

n3wscott commented 7 years ago

I am working on this now.

I am going to recommend we not use swagger, and instead use JSON Hyper-Schema. I am going to work up an example in the next couple weeks.

arschles commented 7 years ago

@n3wscott I am unfamiliar with hyper-schema. I will read the spec when I can, but before I do, a few hopefully quick questions:

Are there tools to generate clients & servers?
Can tools generate a UI of the API? With docs?

I am biased toward Swagger, but if hyper-schema works for our purposes, I'm all good with that too

MHBauer commented 7 years ago

@maximilien we're tracking the swagger/openapi work here.

luksa commented 7 years ago

@dizz note that the definitions in your schema need to include the type: object field, otherwise you're saying that an array or a primitive is also a valid value (for e.g. the Services definition). See https://stackoverflow.com/questions/47374980/schema-object-without-a-type-attribute-in-swagger-2-0

Also:

Services should probably be Service
The definition of ServiceParameters should be something like this:
```
ServiceParameters:
description: Configuration options for the service instance.
type: object
```
(to allow it to have properties with arbitrary names)

dizz commented 7 years ago

Ah you're right @luksa ! Thanks for spotting. Would you be willing to throw over a pull request so you can be credited?

luksa commented 7 years ago

@dizz done. I realized I was not looking at your version of the schema, but @mattmcneeney's. But yours also had the missing type:object problem.

dizz commented 7 years ago

Thanks @luksa

mattmcneeney commented 6 years ago

Closing this as being done in #389

openservicebrokerapi / servicebroker