Skip to content
This repository has been archived by the owner on Dec 21, 2020. It is now read-only.

How to specify "details" in response? #21

Closed
michaz opened this issue Apr 14, 2018 · 9 comments
Closed

How to specify "details" in response? #21

michaz opened this issue Apr 14, 2018 · 9 comments

Comments

@michaz
Copy link
Member

michaz commented Apr 14, 2018

((What I'm writing here is assuming that path details are dynamic -- that the server can invent new types of path details, and the client does not have to be rebuilt to use them. I think that was the intention. If not, ignore me.))

I'm afraid we may have managed to produce an API which is unspecifiable in Swagger.

Path details look like this:

"details":{"r5_edge_id":[[0,1,2772083],[1,2,397628],[2,3,397635],[3,4,397632],[4,5,999487],[5,6,999485],[6,7,999482],[7,8,2772090]]}

This is an object which consists of dynamic properties. This is unsupported in Swagger, except if the object is entirely free-form, meaning that also the schema of the list of arrays int[3] is lost:

        "details": {
          "type": "object"
        }

Any suggestions? Shall we try and be more like "Swagger first" to prevent this?

@michaz
Copy link
Member Author

michaz commented Apr 14, 2018

Or am I getting this wrong and it was never meant to be dynamic, and all currently implemented path details should be listed in the spec as properties of details in the response, and as allowed values of details in the request?

@karussell
Copy link
Member

karussell commented Apr 14, 2018

Yes, it is intended to by dynamic at some point (for now we have a limited list)

Any suggestions? Shall we try and be more like "Swagger first" to prevent this?

I wouldn't bind us to limitations of Swagger (still a 'young' project with many limitations). Instead we should fix swagger (or at least create an issue) if this gets critical (gets dynamic).

@boldtrn
Copy link
Member

boldtrn commented Apr 17, 2018

I agree with @karussell. I think, accepting this as a swagger limitation (and maybe raise an issue there as well) is acceptable. If I understand you correctly, you can still use the client if we specify this as a free object. The client would then only return a map (or a similar structure) for the PathDetails, which I find acceptable for now.

@michaz
Copy link
Member Author

michaz commented Apr 17, 2018

So we are using Swagger not as a specification language, or as a framework that may contain useful guidelines about what may be usual or non-surprising ways to do something, but just as a tool to auto-generate clients if that happens to be possible, and we want to keep it that way? Okay.

@michaz
Copy link
Member Author

michaz commented Apr 17, 2018

Because I was already dreaming of using it to actually design the API, and generate the (only, real, live) documentation and such.

@michaz
Copy link
Member Author

michaz commented Apr 17, 2018

But if it is too limited for us, probably not.

@boldtrn
Copy link
Member

boldtrn commented Apr 17, 2018

Because I was already dreaming of using it to actually design the API, and generate the (only, real, live) documentation and such.

Yes I was dreaming of the same thing :). To be honest I find it easier to write normal documentation and to use swagger only to generate clients, but this does not mean that we shouldn't use Swagger as our documentation tool. I haven't followed the latest developments from Swagger, maybe the documentation part became a lot better?

The biggest problem for me is, if we cannot do something properly in Swagger, we cannot document it properly and then we cannot generate clients from it properly as well. This however does not mean that we shouldn't built a certain feature, if we believe that the way we specified it is good. Swagger cannot simply update their specification, so if we find an issue, we might have to wait for the new Swagger version to be released and is stable to implement a feature, etc.

I 100% support the idea of Swagger and I think these guys are doing an awesome job. And I think Swagger works perfectly well for simple CRUD APIs (maybe even to generate the server code). But some of our optimizations are just really hard to model in Swagger, for example "points_encoded" has been a big issue and I think it's still not possible to do this properly (See #1, and also the related issue here).

@michaz
Copy link
Member Author

michaz commented Apr 23, 2018

But some of our optimizations are just really hard to model in Swagger, for example "points_encoded" has been a big issue and I think it's still not possible to do this properly

I see. But couldn't that suggest that for, say, version 2, we should give Swagger (or another specification tool) a bigger role while designing the API?

Maybe if we would do that, the next version of the points_encoded feature would be realized differently. Say, with differently-named response fields for the two different data-types.

I am still not completely convinced that we know good practices about these kinds of things better than the tools.

@michaz michaz closed this as completed Apr 23, 2018
@boldtrn
Copy link
Member

boldtrn commented Apr 23, 2018

But couldn't that suggest that for, say, version 2, we should give Swagger (or another specification tool) a bigger role while designing the API?

Yes I think it would be good to consider a tool like Swagger when we design the version 2 of the API. If we can create a compatible API.

Maybe if we would do that, the next version of the points_encoded feature would be realized differently. Say, with differently-named response fields for the two different data-types.

Yes, I think this also makes more sense from a consumer-perspective. One key that can contain different content might be more complex to grasp. On the other hand, recent trends with for example NoSQL databases also show that there is a use-case for this.

I am still not completely convinced that we know good practices about these kinds of things better than the tools.

Me neither :).

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

No branches or pull requests

3 participants