swagger files for the servers #1
Conversation
TeodoroFilippini
force-pushed
the
swagger-server-files
branch
from
3435f82 to
7e8dc9f
Compare
There was a problem hiding this comment.
I think you were a bit confused with instances running the pipeline and channels -- the mailgun relayery does not know anything about the instances; it only needs to forward the messages (think of it as a proxy for email).
I hope the comments could clarify it a bit? Otherwise, if it's still not clear, let's have a look at it off-line in more detail today in the office.
Re merging to first-version: good idea!
TeodoroFilippini
force-pushed
the
swagger-server-files
branch
2 times, most recently
from
670792e to
b3f9dd0
Compare
TeodoroFilippini
force-pushed
the
swagger-server-files
branch
from
b3f9dd0 to
169f9fe
Compare
|
@mristin here's the updated Swaggers. LMK what you think! |
swagger/control/swagger.yaml
Outdated
| type: array | ||
| items: | ||
| $ref: "#/definitions/Channel" | ||
| rels: |
There was a problem hiding this comment.
Since you are not including the rels in the header, why not just include the property "pages" here?
swagger/control/swagger.yaml
Outdated
| description: contains a URL. | ||
| type: string | ||
|
|
||
| Rels: |
There was a problem hiding this comment.
Rels are actually expected in the header. Since you are including them in the response body (and not response header), you can do away with OptionalRel and just set prev and next as not required fields, couldn't you?
There was a problem hiding this comment.
You think that's the way to go? Otherwise I can set all rels in the header directly in the code and perhaps only mention their existance in the swagger description of the ChannelsPage.
There was a problem hiding this comment.
Using rels in the headers (hypermedia link relations) is not trivial to implement in the client (and it seems it's not set yet, see OAI/OpenAPI-Specification#688).
I'd go for the easier solution that I suggested since we could already use swagger-to to generate the client(s).
|
@mristin please take a last look at the |
| - per_page | ||
| - channels | ||
| - rels | ||
| - prev |
There was a problem hiding this comment.
Since you already added page_count, I think that prev and next are now obsolete since the client needs to iterate over page_count specifying the page parameter in list_channels endpoint.
This reverts commit b38293f.
@mristin
Let me know what you think of these Swagger files for the two MailGun Relayery servers. I am unsure about the term "instance" to refer to someone pushing to the Forward Server - do you have a better term in mind?
Also, notice this branch merges to first-version and not master; I thought we could split the tool in several PRs to avoid large, blocking code review sessions.