[WIP] Use Cornice Swagger to generate the OpenAPI specification #1033

gabisurita · 2017-01-25T18:38:03Z

This is still early WIP but I need some feedback on the architecture. If you think the changes are "too breaking" we can rollback ASAP and find another approach. The more controversial change IMO is to move parts of the schema from resource to a separate module. I've did this to allow imports from other parts of the code that are imported before Service is defined (like errors), and most of the schemas there are not exclusive to resources. Opinions?

Todos:

Extras (can be finished at other PRs):

Add response details.
Add descriptions everywhere.
Investigate using the response schemas to actually build responses.
Use references to reduce document size (Allow custom references Cornices/cornice.ext.swagger#57)

r? @glasserc

glasserc · 2017-01-27T04:11:12Z

I didn't have time to do a thorough review, but from what I see, this looks OK to me. I don't have a problem with concentrating fundamental or common schemas in a schemas module. (In Haskell, sometimes projects will have a Types.hs module where all the models are defined but most of the operations they support omitted. This seems like that to me.) On the other hand, I like better how you put e.g. VersionResponseSchema in the same module as the view that serves versions -- it's closer, so it seems to me like it will be more likely to be modified at the same time. So I would say that we should try to do that as much as we can, and fall back to putting things in schemas if we have to. At first I thought you put things in schemas to re-use them, but I see that e.g. CollectionRequestSchema is only used once?

The ResourceResponses thing seems a little complicated.. I guess this is to simplify shared error responses? It feels like it's tied too tightly to the different views we support.

glasserc · 2017-01-27T04:11:28Z

I'd love to hear @leplatrem 's thoughts on this!

glasserc · 2017-01-27T04:13:33Z

tests/swagger/test_validation.py

@@ -113,7 +113,7 @@ def test_validate_headers(self):
        for head in headers:
            self.request.headers = head
            self.assertRaises(ValidationError, unmarshal_request,
-                              self.request, self.resources['Buckets'].get_buckets)
+                              self.request, self.resources['Bucket'].get_buckets)


Ouch.. too bad we have to make dozens of changes like this. Maybe we should try to find a few minutes in a subsequent PR to extract this expression as an attribute on the test case, something like self.swagger_resource?

That would be good. I mean, now we are generating it automatically I don't think we should test it this way. We can probably think of something better.

gabisurita · 2017-01-27T12:17:58Z

The ResourceResponses thing seems a little complicated.. I guess this is to simplify shared error responses? It feels like it's tied too tightly to the different views we support.

Hmm, these are actually all the responses for the Resource class, and we are actually changing the record schemas (e.g allowed permissions) for each view during initialization here.

Changing the error responses is not something we would probably do in the server repository, because all storage views have the exactly same error messages. We did the same with the handwritten spec. But there may be some extensions that would like to change it. I just realized we can make responses a Resource attribute rather than a ViewSet attribute, than we could just change the responses messages by setting response_schemas on each Resource derived class.

gabisurita · 2017-01-27T12:26:25Z

So I would say that we should try to do that as much as we can, and fall back to putting things in schemas if we have to. At first I thought you put things in schemas to re-use them, but I see that e.g. CollectionRequestSchema is only used once?

CollectionRequestSchema is used only on resources, and that's why it's on kinto.core.resource.schema. Any and QueryField are reused, and that's why they are on kinto.core.schema. What I did was to move some non resource specific schemas from kinto.core.resource.schema to kinto.core.schema. IMO CollectionRequestSchema should remain in kinto.core.resource.schema.

This is needed to allow importing generic schemas on other parts of kinto core without breaking the import order. Related to Kinto#1033

gabisurita · 2017-02-14T19:25:04Z

Closed in favor of #1078

gabisurita added 17 commits January 25, 2017 11:00

move schemas to a separate module

44cd063

list response schemas

a600187

use new request schemas and add responses to viewset

0480cd0

use error response schema in errors

25e3d40

use cornice swagger to generate documentation

f27f7f2

fix some tests

1faf09e

pep8

d17774e

fix setup file

1d36ba2

include basicauth security

ae4d4e6

simplify getting responses on viewset

774bb11

add missing tags, operation ids and sketch non-resource responses

3d277b4

Attempt to simplify changes ans reduce diff

6194770

simplify and (dummy) fix view tests

831b177

remove definitions tests as they don't make sense anymore

7055e24

fix plugin tests

5099e4a

fix schema tests

4330964

patch changes in errors messages on core

1f71926

glasserc reviewed Jan 27, 2017

View reviewed changes

This was referenced Jan 28, 2017

Set permission fields on initialization rather than request time #1046

Merged

Split request schemas for each method/endpoint type #1047

Merged

gabisurita added a commit to gabisurita/kinto that referenced this pull request Feb 1, 2017

Move generic schemas outside resource

eba6a77

This is needed to allow importing generic schemas on other parts of kinto core without breaking the import order. Related to Kinto#1033

This was referenced Feb 1, 2017

Move generic schemas from Resource to kinto.core #1054

Merged

Add endpoint info in OPTIONS #707

Closed

This was referenced Feb 10, 2017

Nit: simplify HeaderQuotedInteger validator #1065

Merged

Use cornice swagger to generate the OpenAPI documentation #1078

Merged

gabisurita closed this Feb 14, 2017

gabisurita deleted the cornice-swagger branch February 23, 2017 00:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Use Cornice Swagger to generate the OpenAPI specification #1033

[WIP] Use Cornice Swagger to generate the OpenAPI specification #1033

gabisurita commented Jan 25, 2017 •

edited

Loading

glasserc commented Jan 27, 2017

glasserc commented Jan 27, 2017

glasserc Jan 27, 2017

gabisurita Jan 27, 2017

gabisurita commented Jan 27, 2017

gabisurita commented Jan 27, 2017 •

edited

Loading

gabisurita commented Feb 14, 2017

[WIP] Use Cornice Swagger to generate the OpenAPI specification #1033

[WIP] Use Cornice Swagger to generate the OpenAPI specification #1033

Conversation

gabisurita commented Jan 25, 2017 • edited Loading

glasserc commented Jan 27, 2017

glasserc commented Jan 27, 2017

glasserc Jan 27, 2017

Choose a reason for hiding this comment

gabisurita Jan 27, 2017

Choose a reason for hiding this comment

gabisurita commented Jan 27, 2017

gabisurita commented Jan 27, 2017 • edited Loading

gabisurita commented Feb 14, 2017

gabisurita commented Jan 25, 2017 •

edited

Loading

gabisurita commented Jan 27, 2017 •

edited

Loading