redash icon indicating copy to clipboard operation
redash copied to clipboard

RFC: API Documentation

Open arikfr opened this issue 7 years ago • 22 comments

Redash has a very rich API, but it's undocumented[1] which makes it far less useful or adopted than it could be.

It seems that the way to go forward is to adopt something like OpenAPI (f.k.a Swagger) to generate an API spec that we can then use to create the documentation (and maybe clients as well).

In the Python/Flask ecosystem there are several libraries to help with this:

  1. marshmallow -- to define schemas and serialization/deserialization of objects.
  2. apispec to generate OpenAPI specs.

There are also marshmallow-sqlalchemy and several Flask/apispec libraries.


It will be great to get comments and ideas from others who tried these or other tools already before we start implementing this.

arikfr avatar Oct 18 '18 06:10 arikfr

Yep, OpenAPI is the way to go, and I think marshmallow has shown to be a very stable library over the years. The tricky thing is to figure out how to combine marshmallow with Flask-Restful given the fact it has an own request parser and serialization module: https://github.com/flask-restful/flask-restful/issues/335

There is also https://github.com/rochacbruno/flasgger which seems to have support for both Flask-Restful as well as apispec, which could help simplify the implementation. OTOH its support for Flask-Restful is still requiring to write out the OpenAPI spec as YAML in the resources docstring. Moving straight to a different API framework and using marshmallow's Schema would probably be less work.

jezdez avatar Oct 18 '18 08:10 jezdez

The tricky thing is to figure out how to combine marshmallow with Flask-Restful given the fact it has an own request parser and serialization module: flask-restful/flask-restful#335

I don't think we use much of Flask-Restful's serializer, so it's only the request parser that we need to figure out. But I don't mind dropping Flask-Restful at this point. Not sure we get much value from it.

Flasgger looks nice, but if indeed their support for Flask-Restful doesn't allow for using marshmallow, then we should probably skip on it (and Flask-Restful). It might require more upfront work, but definitely will be simpler in the long term.

arikfr avatar Oct 18 '18 08:10 arikfr

The tricky thing is to figure out how to combine marshmallow with Flask-Restful given the fact it has an own request parser and serialization module: flask-restful/flask-restful#335

I don't think we use much of Flask-Restful's serializer, so it's only the request parser that we need to figure out. But I don't mind dropping Flask-Restful at this point. Not sure we get much value from it.

Flasgger looks nice, but if indeed their support for Flask-Restful doesn't allow for using marshmallow, then we should probably skip on it (and Flask-Restful). It might require more upfront work, but definitely will be simpler in the long term.

Yeah agreed, may this be a nice hackweek project?

jezdez avatar Oct 18 '18 08:10 jezdez

Yeah agreed, may this be a nice hackweek project?

Good idea!

Btw, we should also think about API versioning. If there is a way to introduce that without complicating the task, then maybe we should do that too.

arikfr avatar Oct 18 '18 08:10 arikfr

Author of marshmallow and apispec here. Found this via https://github.com/flask-restful/flask-restful/issues/335 . Very cool to see that you're considering marshmallow and apispec for this project!

As you can see from https://github.com/flask-restful/flask-restful/issues/335 , the maintainers of Flask-RESTful are dropping support for reqparse and marshal and are recommending marshmallow in its place. One of the maintainers went so far as to propose deprecating the entire project.

I don't think we use much of Flask-Restful's serializer, so it's only the request parser that we need to figure out.

I recommend taking a look at webargs for request parsing. It has first-class support for marshmallow and therefore integrates nicely with apispec as well.

The "marshmallow stack"--marshmallow, webargs, and apispec--works nicely with vanilla Flask. That said, there are a number of frameworks that provide integration between Flask and the marshmallow libs. Along with Flasgger, there's flask-rest-api (maintained by one the marshmallow co-maintainers) and flask-resty (maintained by a frequent contributor).

sloria avatar Oct 19 '18 01:10 sloria

@sloria Hey, thank you for chiming in, this is super helpful!

jezdez avatar Oct 19 '18 08:10 jezdez

Somewhat related, how DRF handles versioning: https://www.django-rest-framework.org/api-guide/versioning/

arikfr avatar Nov 08 '18 14:11 arikfr

https://github.com/noirbizarre/flask-restplus/

The main thing I liked here is the fact that the API is very similar to Flask-Restful, so migration should be very easy. What I mostly didn't like is that it doesn't use Marshmalow :)

arikfr avatar Dec 03 '18 20:12 arikfr

@arikfr Yeah, there's no built-in support for marshmallow in flask-restplus. Looks like there have been various attempts, based on this warning in the docs, https://github.com/noirbizarre/flask-restplus/issues/9, https://github.com/noirbizarre/flask-restplus/issues/438, https://github.com/noirbizarre/flask-restplus/issues/317 . Not sure if there's active work on the integration, though.

sloria avatar Dec 03 '18 20:12 sloria

Once again, thank you @sloria for chiming in! This is really helpful.

Btw, in your previous comment you didn't mention flask-apispec. Is it because you think flask-rest-api is superior to it or some other reason?

arikfr avatar Dec 03 '18 20:12 arikfr

The maintainer of flask-apispec and I aren't actively working on it any more, so I hesitate to recommend it. flask-rest-api and flask-resty, on the other hand, are actively maintained.

sloria avatar Dec 03 '18 20:12 sloria

OK, makes sense. Thank you for the clarification!

arikfr avatar Dec 03 '18 20:12 arikfr

How is the json validation of request body, and response body in tests?

koooge avatar Jan 17 '19 07:01 koooge

@koooge I'm not sure I understand what you mean?

arikfr avatar Jan 17 '19 08:01 arikfr

Redash's API is so buggy now. Most of them returns 5xx when it is called with unexpected parameters. So, I suppose they need to validate the request body(json) or query strings, then should return 4xx to client. marshmallow(or else) libraries looks good, but can they check such json schema/variable types/required parameters? Besides, Redash handlers' tests should check response body. I think it is useful if we can use the same libs in tests.

koooge avatar Jan 17 '19 15:01 koooge

Yes, Marshmallow (or similar) will help here indeed.

As for the tests, I would look into the option of using snapshot testing for this. But reusing the same lib make sense indeed.

arikfr avatar Jan 17 '19 15:01 arikfr

I haven't formed my opinion about GraphQL yet, but figured it should be mentioned that this is another option for having a documented API while getting some other benefits.

We can even gradually adopt GraphQL alongside our REST API and later decide if we want to deprecate the REST API entirely or have them coexist.

arikfr avatar Jun 23 '19 10:06 arikfr

At work, we're using a Node.js GraphQL API in front of RESTful services written in Python with Flask-RESTy. I don't know enough about redash to recommend this approach for this project, but it's worked very nicely for us. It allows us to write the business logic in Python but also take advantage of the GraphQL ecosystem in JS (which is more mature than Python's ATM).

sloria avatar Jun 23 '19 15:06 sloria

@sloria thanks! Something like this crossed my mind, but I would prefer to keep the amount of tools/services one needs to deploy with Redash to a minimum. We will revisit this if we do adopt GraphQL and see that the tooling in Python is still sub-par.

arikfr avatar Jul 02 '19 12:07 arikfr

We had a discussion about this with the Redash team and decided that we will split this into multiple steps:

  1. Start documenting the API using either just "prose" or some simple tool like API Blueprint, Postman or something that generates OpenAPI specs. The benefit of using a spec instead of just prose is better structure and ability to run automated tests to make sure we don't have mismatch between the docs and the actual API.
  2. The documentation won't cover 100% of the API, but just the main resources/use cases.
  3. We will look into improving the friendliness of the API with better error messages, to allow easier exploration. We can probably achieve some of this by adopting marshmallow or similar for serializing responses and deserializing requests.
  4. [Further in the future] adopt apispec or similar and API versioning.

Comments are welcome. Later on, I will replace this issue with a few separate ones for each step.

arikfr avatar Jul 02 '19 12:07 arikfr

Comments are welcome. Later on, I will replace this issue with a few separate ones for each step.

Following the mammoth task of the React conversion, this seems like a sensible strategy. 👍

jezdez avatar Jul 02 '19 15:07 jezdez

It's 2024 now. No OpenAPI/Swagger spec in sight yet. :)

pkit avatar Sep 10 '24 01:09 pkit