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

Broken examples #267

Closed
maxlinc opened this issue Feb 7, 2015 · 15 comments
Closed

Broken examples #267

maxlinc opened this issue Feb 7, 2015 · 15 comments

Comments

@maxlinc
Copy link
Contributor

maxlinc commented Feb 7, 2015

Several of the examples in this repo are invalid according to:

  • swagger-tools validate
  • The Swagger editor
  • The validator badge service (although I'm not sure it's working)

For example, the uber.yaml example shows these errors in via swagger-tools or the editor:

$ swagger-tools validate examples/v2.0/yaml/uber.yaml

API Errors:

  #/definitions/Activities/properties/history: Missing required property: items
  #/paths/~1products/get/responses/200/schema/items/$ref: Not a valid JSON Reference
  #/paths/~1products/get/responses/default/schema/$ref: Not a valid JSON Reference
  #/paths/~1estimates~1price/get/responses/200/schema/items/$ref: Not a valid JSON Reference
  #/paths/~1estimates~1price/get/responses/default/schema/$ref: Not a valid JSON Reference
  #/paths/~1estimates~1time/get/responses/200/schema/items/$ref: Not a valid JSON Reference
  #/paths/~1estimates~1time/get/responses/default/schema/$ref: Not a valid JSON Reference
  #/paths/~1me/get/responses/200/schema/$ref: Not a valid JSON Reference
  #/paths/~1me/get/responses/default/schema/$ref: Not a valid JSON Reference
  #/paths/~1history/get/responses/200/schema/$ref: Not a valid JSON Reference
  #/paths/~1history/get/responses/default/schema/$ref: Not a valid JSON Reference
  #/definitions/Activities/properties/history/$ref: Not a valid JSON Reference

API Warnings:

  #/definitions/Product: Definition is defined but is not used: #/definitions/Product
  #/definitions/PriceEstimate: Definition is defined but is not used: #/definitions/PriceEstimate
  #/definitions/Profile: Definition is defined but is not used: #/definitions/Profile
  #/definitions/Activity: Definition is defined but is not used: #/definitions/Activity
  #/definitions/Activities: Definition is defined but is not used: #/definitions/Activities
  #/definitions/Error: Definition is defined but is not used: #/definitions/Error

12 errors and 6 warnings
@maxlinc
Copy link
Contributor Author

maxlinc commented Feb 7, 2015

Note: I setup gulp and mocha as part of the travis-ci build to validate examples a while ago. At that time swagger-tools didn't exist - or at least didn't have an easy to use CLI validator for Swagger v2 yet. Now that it's an easy to use, thorough validator that is used by swagger-editor I think it would be good to hook into the Travis build so it validate changes to examples.

@whitlockjc do you think you could hook swagger-tools into the Travis build to validate changes to examples/*? If you don't have time I can send a PR with a naive hookup (i.e. bash for loop), but I'm not a javascript guy so wanted to see if you had an opinion about validation via pure CLI vs something like a grunt/gulp task or mocha tests.

@webron
Copy link
Member

webron commented Feb 7, 2015

@maxlinc - I'm working on fixing the samples, and most of them should be fixed in the 2.0_fixes branch.

Did you encounter the problems with the YAML samples or also the JSON ones? I'm very likely going to remove the whole YAML samples, or possibly move them to the swagger-editor repo as they are really only relevant there.

@maxlinc
Copy link
Contributor Author

maxlinc commented Feb 7, 2015

Please don't remove the YAML examples, they're very useful and not just for swagger-editor. I agree we shouldn't need to maintain both sets, but I think the solution is to write one and then convert it to the other - and validate both.

Personally I'd prefer to write YAML and generate JSON, because YAML is designed designed for humans: it's easier to read, easier to write, and it allows comments. In my opinion writing Swagger in JSON and then converting to YAML is like writing a README.html and then using pandoc to convert it to README.md. Even if you ultimately need HTML it doesn't mean you should write it.

The comments are especially useful for examples, and even more so when you consider tools like groc or docco to turn commented YAML sample into HTML documentation, or a tool I'm finishing up to turn commented YAML into Markdown.

I'm not saying we should document the YAML examples and generate documentation, but here are some examples just to show the power and flexibility of writing examples in YAML.

Adding these comments to the uber example:

# ## Uber Example
#
# This is an example of the [Uber API](http://blog.uber.com/api)
# as a demonstration of an API spec in YAML.
swagger: "2.0"
info:
  title: Uber API
  description: Move your app forward with the Uber API
  version: "1.0.0"
# The `host` should be the production domain of your service. It's recommended
# that [Swagger-compatible tools][tools], especially ones designed for testing
# like [Flex][flex] or [Pacto][pacto] allow you to override the host for testing
# purposes, so you could set it to something like `stage.api.uber.com` without
# editing the Swagger file.
#
# Note the host does not allow [URI templating][templating].
#
# [tools]: https://github.com/swagger-api/swagger-spec#additional-libraries
# [flex]: flex-swagger.readthedocs.org
# [pacto]: https://github.com/thoughtworks/pacto
# [templating]: https://github.com/swagger-api/swagger-spec/blob/master/versions/2.0.md#path-templating

Would be rendered in groc's parallel style for displaying commented YAML as:
screen shot 2015-02-07 at 3 58 57 pm

Other tools have inline styles if you don't like parallel, and inline images are even possible though in my experience they usually don't look great (especially in the parallel style). Links to images are probably better, but here's an example anyways.

This YAML:

      summary: Product Types
      description: The Products endpoint returns information about the Uber products offered at a given location. The response includes the display name and other details about each product, and lists the products in the proper display order.
      parameters:
        - name: latitude
          in: query
          description: Latitude component of location.
          required: true
          type: number
          format: double
        - name: longitude
          in: query
          description: Longitude component of location.
          required: true
          type: number
          format: double
# The parameters defines the parameters that are used in by the operation, and is also used to generate
# the forms in Swagger-UI or Swagger Editor that allow people to try your operation. The parameters above would
# produce this form in Swagger Editor:
#
# ![form](https://cloud.githubusercontent.com/assets/896878/6093974/653be95c-aee3-11e4-917d-2df0443b4c74.png)

Would produce this after it's automatically turned into Markdown and then rendered via GitHub:

screen shot 2015-02-07 at 4 10 40 pm

@webron
Copy link
Member

webron commented Feb 7, 2015

@maxlinc - I really appreciate your comments, but the fact is that the spec is in JSON and putting the YAML samples here make people assume that's not the case. Among other things, this leads to support burden.

I'm not saying there aren't benefits to YAML, but the tools we provide are meant to produce and consume the JSON spec. Swagger-editor uses YAML to provide for easier human manipulation, and that's perfectly fine. That's also the reason why the YAML samples should move there, as they serve that purpose.

I understand you use it in possibly other ways, and that's your prerogative. However, I don't want to mislead potential users into thinking that this is an official part of the spec. Nobody's stopping anyone else from doing what you do as well. Someone may even write their own DSL that would translate to the Swagger JSON, and that's perfectly fine.

@maxlinc
Copy link
Contributor Author

maxlinc commented Feb 11, 2015

Ron,

I appreciate you're trying to avoid a support burden, but when you say "I understand you use it in possibly other ways” you’ve completely misunderstood. I’m actually trying to make sure that I use it in exactly the same way it is used in other Swagger projects by Swagger contributors like @fehguy, @whitlockjc, @mohsen1, @earth2marsh and others. They have all used or proposed YAML support in projects after the Swagger 2 announcement. So I'd like to get their input about where/how to share YAML examples before you remove them from this repo. You may feel that swagger-editor is the obvious choice, but YAML support actually comes from its dependency, swagger-tools.

I also think you may have some misconceptions about YAML that I hope to clear up. You asked a good question on another issue where @fehguy and @earth2marsh were in favor of YAML support in swagger-core and swagger-jaxrs:

@fehguy: Ron the format for the spec and tooling is JSON. There is no reason why we can't produce yaml though
@webron: We can also produce XML and HTML directly from it, but there's no reason to do those either.

Please allow me to answer the question “why YAML, why not XML?” I’ll try to keep it brief.

(Edit: I failed)

YAML was an explicit goal

Why support YAML at all? Because when the Swagger working group was announced it stated:

The aim of 2.0 is to add naming, an API design-first workflow using a YAML syntax and editor, extend the metadata, and use best of breed tooling.

I don't think that was a random goal. I believe a large percentage of the community wanted YAML and is very happy that Swagger 2 has embraced YAML. YAML describes JSON as "trivial to generate and parse, at the cost of reduced human readability." I think runtime projects may choose to only support JSON because of the simplicity, but design-time projects should consider supporting YAML because of it's human readability/writability.

In my view design-time isn't limited to the editor. I use a variety of design techniques including spikes, prototypes, and test-driven design. In my ideal world developers, QAs, and technical writers should all be able to collaborate on the same (YAML) document. The QAs may be focused on adding examples, the devs on refining the schemas, and the technical writers on reviewing the overall documentation is complete, well-written, and grammatically correct. They should all be collaborating on the same document, not constantly trying to switch back and forth between JSON and YAML just because the writers are using the editor while devs and QAs use other tools.

In fact, I would say that the API definition should remain in YAML until the "last responsible moment". That may mean it could always remain in YAML, or it may mean it needs to be converted to JSON just before loading into a runtime project that doesn't support YAML.

I'd also love to use the YAML design-first workflow for writing examples for this spec. It's eating our own dogfood, isn't it?

JSON is YAML

YAML stands for “Yet Another Markup Language” and I never realized what a horrible name that is. It should have been called “JSON Markup Language” because it is so closely related to JSON. This is in the YAML specification states:

YAML can therefore be viewed as a natural superset of JSON, offering improved human readability and a more complete information model. This is also the case in practice; every JSON file is also a valid YAML file. This makes it easy to migrate from JSON to YAML if/when the additional features are required.

Since “every JSON file is also a valid YAML file”, it means that YAML doesn’t require a separate specification. Technically it doesn’t even require separate examples (although I feel YAML examples are more readable/writable than JSON). If anything the YAML specification would just be an exact copy of the JSON specification with a couple of notes about restricted syntax. In practice there are no real syntax restrictions. The only YAML features that can’t be cleanly converted to JSON are:

  • Comments: but comments are intended to be thrown away
  • Custom data types: this is not a commonly used feature, and since allowable types are clearly stated throughout the Swagger specification there should be no expectation these are allowed

YAML support is trivial

It’s trivial to convert between YAML to JSON. That’s why there is a service online (and in every language). So I figure it’s pretty easy for the spec to say:

Swagger parsers MUST support loading Swagger documents from JSON. They MAY support loading Swagger documents from YAML by first converting it to JSON.
Swagger producers MUST produce JSON or YAML that can be converted to JSON.

That’s actually the strict version. Usually JSON and YAML and YAML are parsed to the same primitive data types anyways (unlike XML & HTML when are parsed into objects of XML/HTML libraries) so it often isn’t even necessary to convert YAML to JSON - you just load either YAML or JSON and the result is the same.

Here is the code inside swagger-codegen (as part of yml2swagger), swagger-parser, swagger-tools, my projects swagger-rb and wad2swagger, and similar code (for apparently a different purpose) in swagger-core.

My point is that developers don’t need to support YAML but if they choose to do so it’s only a few extra lines of code.

Who is the spec for?

You write:

that the spec is in JSON and putting the YAML samples here make people assume that's not the case

Which people? Is this spec for Swagger users or Swagger developers?

If you're talking about Swagger users than you've already confused them by promoting YAML as a format for designing APIs with Swagger and the pointing them towards a documentation that avoids YAML.

If you're talking about developers creating libraries to parse Swagger, which means that this repo has a very small audience, then I think any developer who would be confused by having both JSON and YAML samples shouldn't be writing a Swagger parser in the first place. If they're confused by that then they don't have much shot at implementing the rest of the spec.

Summary

YAML is important to the Swagger community and is used by multiple swagger group and third party projects. I don't believe YAML support should be defined by or limited to the editor, but "design-time" projects in general. The specification in this repo is sufficient to describe both the JSON and YAML syntax, but if you want to remove the YAML examples then it should be made clear that this repository is only for developers, and a separate Swagger Design guide should be created to show people examples of how to design Swagger APIs using YAML.

@earth2marsh
Copy link
Member

Wow, @maxlinc, well said—thank you for such a passionate and thorough articulation. 🌟

@webron I can sympathize with your point of view, but I do tend to prefer YAML for examples because:

  1. I find it a more usable format for authoring, which is generally where my workflows start
  2. Since it is a super-set of JSON, it is easier to go from YAML to JSON than the other direction (most notably with comments, which are very helpful in the design process)

@whitlockjc
Copy link
Member

Minor Nit (For Posterity)

@maxlinc said: You may feel that swagger-editor is the obvious choice, but YAML support actually comes from its dependency, swagger-tools. This is wrong. Since swagger-editor supported only YAML and people were using swagger-editor to create Swagger 2.0 documents, like in apigee-127, the number of YAML based Swagger 2.0 documents were growing in issue reports, examples, frameworks, ... To be able to easily validate those without requiring the user to convert to JSON, I updated swagger-tools to support YAML. So swagger-tools did not dictate YAML support in swagger-editor, it was the other way around. Simple mistake but it might be important if we're trying to figure out what/who is influencing YAML's adoption for Swagger 2.0 document authoring.

My Opinion on YAML

I personally think YAML as an/the official Swagger documentation format makes a lot of sense. The ability to reuse/reference structures, the human readability, comments and more make YAML superior to JSON when it comes to how it's being used for Swagger documents. I would much rather author Swagger documents in YAML than JSON any day.

My Opinion on YAML Support

When it comes to YAML for Swagger 2.0, @fehguy mentioned that first-class support for YAML as a Swagger document authoring format was one of the design goals. Either we didn't read that post or we forgot it but no matter how, we forgot that goal and here we are. (I said we on purpose because I don't remember anyone pointing this out in any recent threads about this topic.) Without having this mind set, I can see where @webron is coming from because as similar as YAML and JSON are, supporting another format officially is not just based on the simplicity of a YAML<>JSON conversion. (As a Swagger tooling author, I know how simple it is to do the whole YAML<>JSON conversion. But at the same time, it was yet another thing I had to do and it was just different enough that it was not a non-issue. In fact, if you look at swagger-tools, the API part of swagger-tools does not support YAML nor does it need to. The CLI supports YAML, converts it to a JavaScript object and then uses the swagger-tools API and goes about its merry way. Everything else (API and middleware) expect/use JavaScript objects.)

Conclusion

In the end, there is no debate on official YAML support for Swagger 2.0 because it technically has been planned from day one. What we need to do now is make it happen in a way that doesn't create the confusion @webron is concerned about. I think this will require consistency. To achieve this, we will need to agree on the approach to document examples in the Swagger specification document itself and what format(s) we use for example Swagger documents. (I do like @maxlinc's idea of using YAML for documentation and examples because with that one source of truth, you could generate equivalent JSON and you could document within your examples since YAML allows you to have comments. But at the same time, I can see @webron's concern about YAML not being as well-known as JSON is...something to think about.) I would love to help out with this effort so that all of this does not fall on @webron, including but not limited to @maxlinc's suggestion of hooking up validation in TravisCI/elsewhere.

@whitlockjc
Copy link
Member

We also need to make sure we tell the community that we're not looking to onboard official support for the plethora of languages that can describe structured data which can adhere to the Swagger specification. I mean, the last thing we need are more requests to officially support languages like Markdown, XML, etc. all because they can be easily converted to JSON. ;)

@maxlinc
Copy link
Contributor Author

maxlinc commented Feb 11, 2015

I would love to help out with this effort so that all of this does not fall on @webron, including but not limited to @maxlinc's suggestion of hooking up validation in TravisCI/elsewhere.

Could you move the code to turn a YAML doc into a JSON doc from the editor into tools? It seems to be a good fit there, especially since you've already got swagger-tools convert for turning 1.2 into 2.0 docs.

That'll do make it easier to:

  • Validate and then convert YAML examples to JSON examples in this repo (or any other repo).
  • Document the use of swagger-tools as a preferred way to convert YAML to JSON for projects that only support loading JSON.

swagger-tools did not dictate YAML support in swagger-editor, it was the other way around

I was unclear. I just meant that the code for supporting YAML is in tools rather than the editor, I wasn't making a claim about why it was there, just that it should have some YAML samples to test YAML parsing/validation.

But it doesn't sound like a major point of contention since we're both in favor of keeping YAML samples in this repo.

the last thing we need are more requests to officially support languages like Markdown, XML, etc. all because they can be easily converted to JSON

I agree we don't need them and can add a disclaimer if necessary. Though I disagree that there is an easy (standard) way to convert those formats to JSON ;)

@whitlockjc
Copy link
Member

Could you move the code to turn a YAML doc into a JSON doc from the editor into tools? It seems to be a good fit there, especially since you've already got swagger-tools convert for turning 1.2 into 2.0 docs.

The code is trivial by use of a Node.js module (yamljs) and here is how I'm doing it in swagger-tools: swagger-tools/blob/master/bin/swagger-tools#L51. What if I made it where you could use swagger-tools convert to convert between JSON and YAML without "upgrading" the document? Then you could easily take a JSON or YAML document and convert it to the other.

I was unclear. I just meant that the code for supporting YAML is in tools rather than the editor, I wasn't making a claim about why it was there, just that it should have some YAML samples to test YAML parsing/validation.

Gotcha. No sweat either way, just clearing things up for those that weren't sure.

I agree we don't need them and can add a disclaimer if necessary. Though I disagree that there is an easy (standard) way to convert those formats to JSON ;)

I completely agree. It was more as a disclaimer than anything because once we open the door, we've no idea what people will ask for.

@whitlockjc
Copy link
Member

Maybe piggybacking on swagger-tools convert isn't ideal, maybe swagger-tools format or something like that. I'll await feedback.

@maxlinc
Copy link
Contributor Author

maxlinc commented Feb 11, 2015

you could use swagger-tools convert to convert between JSON and YAML without "upgrading" the document... Maybe piggybacking on swagger-tools convert isn't ideal, maybe swagger-tools format

Yep, that's exactly what I meant. I don't care too much about the CLI command names, but maybe renaming the existing convert to upgrade and then use convert for YAML->JSON. It might cause some minor short-term confusion but I think those names are clearer.

@whitlockjc
Copy link
Member

I'll file an issue in swagger-tools if you don't beat me to it.

@ePaul
Copy link
Contributor

ePaul commented Apr 19, 2016

Is this fixed now with #389?

@philsturgeon
Copy link
Contributor

Seems like we fixed it. If not, lets reopen, but its been 3 years.

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

No branches or pull requests

6 participants