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

Switch RPC API to sphinx-openapi #1662

Merged
merged 4 commits into from
Sep 30, 2020
Merged

Conversation

achamayou
Copy link
Member

Since #1612 turned /api into an OpenAPI schema, we probably want to use this in the doc. There seems to be two main ways to do that:

  1. Using some sort of ReDoc-style three-pane UI. That can be semi-integrated in Sphinx, but not all that well (cross-linking or search won't work for example).
  2. Inline in Sphinx. This look quite as shiny, but cross-links, search etc do work as expected, which is possibly nice enough that we overlook the lack of shiny and/or contribute some of it to this project.

The app and node APIs work ok with the second option, but the governance schema doesn't at the moment:

Exception occurred:
  File "/data/amchamay/CCF/doc/env/lib/python3.8/site-packages/sphinxcontrib/openapi/openapi30.py", line 108, in _parse_schema
    schema_type = schema.get('type', 'object')
AttributeError: 'list' object has no attribute 'get'

Also strangely, the results schemas themselves aren't rendered, although examples do work:

image

We would need to filter out the logging app entries in the app schema, and work out a way to set a better base URL.

My feeling is that with a bit of tweaking, this could be made to work, but the goal of this draft is really to start a discussion first.

@ccf-bot
Copy link
Collaborator

ccf-bot commented Sep 28, 2020

openapi_sphinx@13415 aka 20200930.30 vs master ewma over 50 builds from 12799 to 13408
images

@achamayou
Copy link
Member Author

The issue seems to come from:

      "uint64_to_Script": {
        "items": {
          "items": [
            {
              "$ref": "#/components/schemas/uint64"
            },
            {
              "$ref": "#/components/schemas/Script"
            }
          ],
          "type": "array"
        },

items ought to be either an object describing a single type, or more probably a "oneOf" in this case.

@achamayou
Copy link
Member Author

      "uint64_to_Script": {
        "items": {
          "items": {
            "oneOf": [
              {
                "$ref": "#/components/schemas/uint64"
              },
              {
                "$ref": "#/components/schemas/Script"
              }
            ]
          },
          "type": "array"
        },
        "type": "array"
      },

Seems to work.

@eddyashton
Copy link
Member

@achamayou: The issue with the gov schema is the known incompatibility between OpenAPI v3.0 and JSON schema, and this is the one instance we have of an incompatible type. I'd intended to nest this in a oneOf but didn't get round to it - I'll do that now, and make this valid-OpenAPI for now.

@achamayou achamayou changed the title Test sphinx-openapi Switch RPC API to sphinx-openapi Sep 30, 2020
@achamayou achamayou marked this pull request as ready for review September 30, 2020 13:02
@achamayou achamayou requested a review from a team as a code owner September 30, 2020 13:02
@achamayou achamayou added the 0.14 label Sep 30, 2020
@achamayou achamayou merged commit 6b2cb92 into microsoft:master Sep 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants