-
Notifications
You must be signed in to change notification settings - Fork 9.1k
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
added notes for additional issues #640
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,133 @@ | ||
# OpenAPI Specification | ||
|
||
### DO NOT MERGE | ||
|
||
These are comments for discussion and should be turned into separate PRs | ||
|
||
#### NOTES | ||
|
||
## Both HTTP and HTTPS ports cannot be specified #214 | ||
|
||
Proposal: We have schemes, host (+ port), basePath, all which are typically linked to gether. Suggest we produce a new object: | ||
|
||
```yaml | ||
hosts: | ||
- scheme: http | ||
host: foo.com | ||
port: 8080 | ||
basePath: /bar | ||
- scheme: https | ||
host: foo.com | ||
port: 8443 | ||
basePath: /baz | ||
``` | ||
|
||
And remove the separate `host`, `basePath`, `schemes`. Because the port will never be the same between http, https, it makes no sense to support multiple ones | ||
|
||
Fixes #489, #214 | ||
|
||
|
||
## Why don't host and basePath support path templating? #169 | ||
|
||
- [ ] Proposal: this belongs under parent #574 | ||
|
||
## Support an operation to have multiple specifications per path (e.g. multiple POST operation per path) #182 | ||
|
||
- [ ] Proposal: this is a payload issue, move to parent #586 | ||
|
||
## Allow for overloading/aliasing of route path #213 | ||
|
||
- [ ] Proposal: change in #70 would address this | ||
|
||
## Allow references between operations inside descriptions. #239 | ||
|
||
- [ ] Proposal: this is really about _links_ and belongs in the #586 issue | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is about links in the documentation, not about links in the payload. I don't see how this relates. |
||
|
||
## Swagger.next: Let's not use hashes anymore #208 | ||
|
||
- [ ] Proposal: while true, moving to arrays causes more problems than it solves. Suggest we close this with no change. | ||
|
||
## Group multiple parameter definitions for better maintainability #445 | ||
|
||
- [ ] Proposal: close. Using global parameters will help with this, and references to arrays of parameters is just wrong.a | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can easily support it with the new construct proposed at #663, and it would make people happy without complicating things. Have parameterGroup as an array of parameters or refs to parameters. Only thing to decide is what happens with conflicts. Granted, it may be a bit more challenging for tools. |
||
|
||
## Allow documenting HATEOAS APIs (pathless operations) #577 | ||
|
||
- [ ] Proposal: do not support pathless operations. It's against the spirit of the specification as being declarative. The support for links _inside_ a path though, will be covered by #574 and #586. Remove reference to #560 and link to others. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As this is my proposal, I'd like to get a bit more explanation for your "against the spirit" judgement. How is an operation declaration (without declaring at which path such an operation might be available) not declarative? (I can see that generating a server implementation from such a definition is difficult, but that is a different reasoning.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the server is self-documenting and has no predefined paths, you don't need OAS IMHO. "/" is a root path. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (I guess we should discuss this at #577 instead of here?) |
||
|
||
## Message format 'wrappers' #555 | ||
|
||
- [ ] Proposal: Move discussion to parent issue #586, this is really about a payload definition, which is covered by that meta issue. Non-structural. | ||
|
||
## Adding optional SLA definitions with the Spec #541 | ||
|
||
- [ ] proposal: Add an optional SLA to the specification, follow the same locations / hierarchy as consumes/produces. Need a SME to take this on and propose a structure. | ||
|
||
## No provision to group API's and then $ref it to reference it in other file #508 | ||
|
||
- [ ] Proposal: I believe what this issue is about is to essentially include an entire path definition into a `path`. This isn't possible currently because PIOs are in a hash, and you can not have a $ref in it. Here is a proposed solution: | ||
|
||
```yaml | ||
paths: | ||
/pet: | ||
get: | ||
... | ||
/health: | ||
$ref: '#/x-paths/health' | ||
'#/x-paths': {} | ||
|
||
# note the location `x-paths` is just illustrative, it could be anywhere allowed by the schema | ||
x-paths: | ||
/store: | ||
get: | ||
... | ||
/users: | ||
get: | ||
... | ||
``` | ||
|
||
treat any fragment or file reference as a "merge" against the root paths object. It resolves like this: | ||
|
||
```yaml | ||
paths: | ||
/pet: | ||
get: | ||
... | ||
/health: | ||
$ref: '#/x-paths/health' | ||
/store: | ||
get: | ||
... | ||
/users: | ||
get: | ||
... | ||
``` | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This requires further discussion. We might as well just allow $ref’s, and multiple of them. We already break the $ref today with the Operation Object (and there’s currently no issue to change that). I’m generally in favor of breaking from $ref’s restrictions, and it makes for a much cleaner solution. If the concern is about $refs disappearing for being multiple keys, we can also have a single $ref with an array of strings. We can even call it something else like Using JSON Pointers as keys is odd, and it’s actually not a JSON Pointer if I understand correctly the usage, it’s the value of a JSON Reference which is not the same. Seems hack-ish and would be a pain to document in the spec. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please propose an alternate solution then |
||
## Clarify when consumes/produces take effect #581 | ||
|
||
- [ ] Proposal: document it, following the HTTP specification for when content-type and accepts headers are acceptable | ||
|
||
## Resource types / patterns for Path Item Objects #588 | ||
|
||
- [ ] Proposal: Ignore it. I don't think we want implicit operations (get|put|post|delete) on a path, consider for 3.1 | ||
|
||
## Allow implicit path parameter matching #132 | ||
|
||
- [ ] Proposal: close. This would be addressed by proposal for #341 | ||
|
||
## Support for multiple request/response models based on headers #146 | ||
|
||
- [ ] Proposal: this is a payload issue, move to parent #586 | ||
|
||
## Global/common parameters #341 | ||
|
||
- [ ] Proposal: Add parameters at the root of the schema. Incompatible parameters will be ignored. Duplicate parameters will use the lower level (i.e. operation) parameter definition. There is no "ignore" parameter mechanism. | ||
|
||
|
||
#### END NOTES | ||
|
||
|
||
#### Version 3.0 | ||
|
||
The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this document are to be interpreted as described in [RFC 2119](http://www.ietf.org/rfc/rfc2119.txt). | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fehguy Please also consider #641 since it's tightly connected to this change.