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

[WIP] Add Open API schema generation. #6119

Closed
wants to merge 23 commits into from
Closed

Conversation

carltongibson
Copy link
Collaborator

@carltongibson carltongibson commented Aug 9, 2018

Right, let's get this show on the road... (Putting up now for early feedback)

We're currently working towards moving to using OpenAPI as our default schema output. We'll also be revisiting our API documentation generation and client libraries.

We're doing some consolidation in order to make this happen. It's planned that 3.9 will drop the coreapi and coreschema libraries, and instead use apistar for the API documentation generation, schema generation, and API client libraries.
-- v3.8 release announcement — What's Next

Work targeting OpenAPI Specification Version 3.0.1 (Is that what we want?)

Overview

  • Introduces OpenAPI versions of the schema generator and individual view inspector classes.
  • Adds a DEFAULT_SCHEMA_GENERATOR_CLASS setting.
  • Adds a generate_schema management command to output serialised schema to stdout. Closes Schema generation - management command #5839

To use this adjust your settings to use the OpenAPI generator/inspector classes:

REST_FRAMEWORK = {
    'DEFAULT_SCHEMA_CLASS': 'rest_framework.schemas.inspectors.OpenAPIAutoSchema',
    'DEFAULT_SCHEMA_GENERATOR_CLASS': 'rest_framework.schemas.generators.OpenAPISchemaGenerator',
}

Any views where you've set an AutoSchema subclass will need adjusting.

Status: WIP

Management command has no subtlety at all (as yet).

Schema generation is stubbed to begin. We generate the paths and operations, but with no details (parameters, responses, etc). Nor do we have the other top-level keys that are available/required.

TODOs

  • Refactor/Tidy new/existing code and tests.
  • Management command
    • Options (?)
    • Tests
  • Complete Open API schema (see below)
  • Documentation.
  • Deprecate CoreAPI schemas
  • Deprecate internal docs, in favour of...
  • Document migration to...
  • Deprecate dynamic schema view (in favour of static) (?)

Help Wanted

I'm no expert on Open API. We need to fill in the top level keys and add the operation details. We need to make sure we're producing the correct output.

It's just a question of filling in the relevant parts of get_schema() and get_operation() but it's details.

All input here welcome!

@codecov-io
Copy link

codecov-io commented Aug 9, 2018

Codecov Report

Merging #6119 into master will decrease coverage by 0.21%.
The diff coverage is 73.22%.

@@            Coverage Diff             @@
##           master    #6119      +/-   ##
==========================================
- Coverage   96.19%   95.98%   -0.22%     
==========================================
  Files         128      133       +5     
  Lines       17706    19697    +1991     
  Branches     1464     1797     +333     
==========================================
+ Hits        17033    18906    +1873     
- Misses        465      576     +111     
- Partials      208      215       +7

@carltongibson
Copy link
Collaborator Author

carltongibson commented Aug 9, 2018

  • Complete Open API schema (see below)

4c17319 adds the first/basic version of get_path_parameters().

The way forward here should be fairly simple:

  • Add the same for serialisers, filters and pagination.
  • Improve the parameter definition, adding components from the Open API spec. (e.g. Schema Objects etc.)

It's here that any OpenAPI experts could really help, by inputting on the format that we need.
(Bonus points for any PRs against this branch. Happy to advise/discuss implementations.)

self.stdout.write(rendered_schema)

def _get_generator_class(self):
return api_settings.DEFAULT_SCHEMA_GENERATOR_CLASS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could drop this - not obv that we need to introduce another setting, or support corejson output from the schema generation command.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. This was just the lazy first option™. 😄

Setting could be moved to a flag on the command. (Assuming we did want to support CoreAPI.)

(Any SchemaView type thing could be passed the generator class as an argument, as I think is already possible.)

@tomchristie
Copy link
Member

Great, thanks @carltongibson.

I'll plug away on the apistar command line tool, and have a look at how well the docs generation there works together with the output we're generating here.

@axnsan12
Copy link
Contributor

It's great to see this issue finally receiving interest and development effort! 😄

Just chiming in with my 2 cents:

Work targeting OpenAPI Specification Version 3.0.1 (Is that what we want?)

Yeah, this is definitely the way to go, as OpenAPI 3 is a superset of OpenAPI 2 and there are tools to convert between the two - so implementing v3 implicitly gets you v2.

On the implementation side, feel free to copy all you want from my code (https://github.com/axnsan12/drf-yasg/tree/master/src/drf_yasg); it's not very cleanly designed but it works. Unfortunately it targets OpenAPI 2 since I haven't had the time to port it over, but a lot of stuff should still be reusable.

I might also be able to help once the general design goals and direction are established, if I find the time.

@carltongibson
Copy link
Collaborator Author

HI @axnsan12. Thanks for commenting. Your help would be greatly appreciated.

...once the general design goals and direction are established...

  • As before, keep everything in schemas. (Rather than adding anything to APIView.)
  • Try to use simple dictionaries, rather than a type abstraction library (like coreapi)
  • Build out from the core/important parts to the less so.

At the moment, the next phase is filling in the operation parameters (serialisers, filters, pagination, etc) and other top-level items in the schema.

I think for the first pass we'll be happy with the 80% being auto-generated. (With users being able to augment that if they need to.) My main issue is that I'm not invested (as yet) in the Open API world, so I don't know which bits of the spec are that important, and which are not.

@n2ygk
Copy link
Contributor

n2ygk commented Sep 9, 2018

Have you considered parsing the OAS schema to scaffold DRF urlpatterns, views, serializers, models, etc.? One can model and mock using, e.g. https://github.com/swagger-api/swagger-editor. Maybe this code generation belongs more appropriately in https://github.com/swagger-api/swagger-codegen.

We are building out a new suite of microservices using DRF+DJA and have been tossing around the idea of creating an OAS 3.0 spec library(?) for JSON:API. I had played around with this briefly for RAML 1.0 before that kind of ended.

@tomchristie
Copy link
Member

Working on a bunch of OpenAPI docs/validation/client libs/mocking/code gen alongside this, so yeah it’s possible that we’ll end up covering the code gen for an initial set of REST framework scaffolding.

@n2ygk
Copy link
Contributor

n2ygk commented Sep 15, 2018

I was taking a look at the metadata API and wonder if this is where you envision an OPTIONS request at the API Root would return a full OpenAPI Object (e.g. with a new OASMetadata(BaseMetadata) class). And maybe return just the pathItemObject for OPTIONS of a path.

I'm trying to think ahead to how we might extend this in in DJA to return an OAS representation of a JSON:API schema.

(See also this jsonapi.org discussion)

@carltongibson carltongibson mentioned this pull request Sep 17, 2018
24 tasks
@tomchristie
Copy link
Member

@n2ygk "maybe return just the pathItemObject for OPTIONS of a path." Yeah we might choose to do that at some point, tho I doubt we'd expose the full schema for OPTIONS on the root (rather just expose the schema explicitly at a given endpoint)

Slight aside that exposing any content in OPTIONS requests is something we might want to default against, so that we can authorize them more liberally than other request methods in order to better support CORS.

@tomchristie
Copy link
Member

Somewhat related to this work is encode/apistar#624, which is essentially...

"APIStar should just be a framework-agnostic API toolkit for generating API documentation, using as an API client, etc..."

So, REST framework will move towards generating OpenAPI directly. (rather than having an internal document model, and generating CoreJSON) and we'll also have a set of tooling that takes over from coreapi, that can validate the schema, build API docs from it, act as an client library, etc...

@n2ygk
Copy link
Contributor

n2ygk commented Oct 2, 2018

@tomchristie This is perhaps not the right place to ask this, but is there a thought to having the OAS/apistar client "look like" a Django Model in a manner similar to https://github.com/Yupeek/django-rest-models? I really want to be able to layer stacks of DRF/DJA microservices on top of each other with perhaps a bottom tier that is database-model based. It seems that this is not easily done today -- unless I'm hugely missing something (entirely possible).

See also: django-json-api/django-rest-framework-json-api#472

@carltongibson
Copy link
Collaborator Author

@n2ygk The discussion group is the best place to take such questions. Thanks!

@onegreyonewhite
Copy link
Contributor

Guys, please add the ability to set custom properties to the fields of serializers. In OpenAPI, this is supported via x-.

@tomchristie
Copy link
Member

@onegreyonewhite Our first pass (here) is just adding a built-in renderer for OpenAPI. This means it's still based on the coreapi document model, so won't directly support x- extensitions. However you will be able to dump the schema with the generateschema command, and then add in any extra annotations directly.

It's likely that for our next major release we'll plan to drop coreapi entirely, and just write out the OpenAPI spec directly. It's far more likely that we'll include a way to add extra annotations then.

@tomchristie
Copy link
Member

@carltongibson Closing this one of for now, given #6229. Doesn't mean the work on it is defunct tho, just that we'll revisit it for 3.10 - most likely pulling in a bunch of stuff from your work here.

@tomchristie tomchristie closed this Oct 5, 2018
@tomchristie tomchristie deleted the 39/update-schemas branch October 5, 2018 09:16
@carltongibson
Copy link
Collaborator Author

No problem!

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

Successfully merging this pull request may close these issues.

None yet

7 participants