python-discord / snekbox

Easy, safe evaluation of arbitrary Python code
https://pythondiscord.com
MIT License
209 stars 38 forks source link

API Documentation #148

Open MarkKoz opened 2 years ago

MarkKoz commented 2 years ago

I'm not happy with the current state of documentation for the API, as it requires looking through docstrings in the source code. I've been considering using an OpenAPI specification and generating a website with documentation using SwaggerUI or ReDoc.

Because snekbox is intended to be hosted by the users, I believe users would find the documentation more convenient and accessible if hosted on GitHub pages. The alternative would be to add a route to Falcon, but this would require users to be able to access their own snekbox instance during development, or temporarily create one.

There are three main approaches to this:

  1. Manually write an OpenAPI spec and manually ensure the Falcon implementation is compliant.
  2. Generate an OpenAPI spec from the code.
  3. Generate Falcon code from an OpenAPI spec.

I'm not commited to using OpenAPI. If there's an alternative for generating nice REST API documentation, I'm open to suggestions.

Approach 1

The upside is that there are no additional dependencies and no code changes requires in snekbox. The downside is that there are 2 things to maintain instead of one. I don't mind that too much though. The bigger concern is making sure they are in-sync. Since the code and spec are in separate files, it seems more easy for them to go out of sync. If there is a tool that can automatically verify an implementation is compliant with a spec, I would be more inclined to go for this option.

Approach 2

The main concern is how to access the OpenAPI spec from the GH Pages repo. The spec could either live on this repo or on the GH pages repo; not sure which makes more sense. CI could generate the spec and then create a commit if it detects a change, but that feels hacky. Alternatively, it could just check if a spec needs to be regenerated without actually doing it, but it would be a bit tedious for the PR author to have to generate it.

The other concern is that it feels backwards to generate a spec from code. Usually it's the other way around: code is written based on a specification. However, I think the OpenAPI spec is more of a means to an end to get some nice documentation. I don't think there is value in the spec itself e.g. for someone to implement the same API in another language.

spectree

It's designed with the intent of documentation being added as a route to the existing API. However, it is possible to avoid adding the route and instead exporting the spec for use elsewhere. It is heavier on dependencies since it mandates the use of pydantic, though it would replace jsonschema since spectree supports validation too. Maybe it evens out.

falcon-apispec

Its intent is to directly generate the spec. It is lighter on dependencies, but I don't like its approach of relying on YAML in docstring.

It doesn't support Falcon 3 and seems unmaintained, but there is a PR that supposedly fixes this. Snekbox technically doesn't need any features of Falcon 3.0, but I don't want to downgrade to an unmaintained version. Furthermore, I don't want to use a library that's unmaintained.

Approach 3

This avoids the problem of generating a spec, since it will be manually written. However, generating code from the spec does feel a bit hacky, and I'm not sure how robust falcon-openapi is. It doesn't seem maintained. Unclear if Falcon 3 is supported (if it is, it probably uses things that are deprecated). This statement from the library also doesn't instil confidence:

I am unsure if operationId will make it into the final version. I may change this to only check for the x-falcon property. I plan on doing more research to determine if this an appropriate way to use the operationId property.

HassanAbouelela commented 2 years ago

I like solution 2, as it seems like the most robust version. We’re not doing anything risky like generating the code from the spec, and we aren’t doing anything that’ll go out of sync. I don’t mind which library. For the GitHub pages aspect, it’s actually a very common thing to deploy the built docs from actions. There’s an action which abstracts 100% of the work, and pushes to a docs branch. We use it on botcore for the docs.

ChrisLovering commented 2 years ago

Yea, solution 2 is nice. We could have some CI that builds the OpenAPI spec, and if it detects what is in the code base is out of date it fails the CI and tells the user to run poetry run task api.

Then another CI flow to push the built spec to GH pages on push to main.

MarkKoz commented 2 years ago

Okay, I was leaning towards 2 as well. In fact, I contributed to spectree to reduce the amount of dependencies it needs.

Can you go into more detail on the build process for GH pages? Would the action on snekbox build the docs and deploy them to the GH pages repo, or would there instead be an action on the GH pages repo to do this?

How would I avoid building docs if there are no changes? Would I need to put the spec file in version control for that? Is it even worth to try to avoid redundant builds?

ChrisLovering commented 2 years ago

We could have the first part of the CI in a custom pre-commit hook if it's performant enough to not be annoying

ChrisLovering commented 2 years ago

How would I avoid building docs if there are no changes? Would I need to put the spec file in version control for that? Is it even worth to try to avoid redundant builds?

Do we even need to worry about that? If unsure what the performance of building these docs is, so we could just build them all the time, if anything in /snekbox is changed if it's only a few seconds.

MarkKoz commented 2 years ago

Apparently redoc just needs a URI to the spec file. Everything else is generated by JS I guess. So, it comes down to

CI could generate the spec and then create a commit if it detects a change, but that feels hacky. Alternatively, it could just check if a spec needs to be regenerated without actually doing it, but it would be a bit tedious for the PR author to have to generate it.

And not sure in which repo the spec file should be.

https://github.com/Redocly/redoc#tldr-final-code-example