-
-
Notifications
You must be signed in to change notification settings - Fork 745
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
[RFC] [PROPOSAL] API version 2 #2686
Comments
I like swagger. Good community around it, I would also encourage us to look at RAML |
I wonder if we need REST framework at all. At its core, API service is very simple: each request goes through a multiple levels of middlewares that translates incoming data into user-friendly api (check cache headers and return proper code if data haven't changed, validate request body and respond with error if it's malformed, parse cookies into list) then gets routed to the particular controller, forms into a response and gets sent back though the another bunch of middlewares (return proper error code if controller throws an error, serialize controller return object into json, add proper headers, cache response). Pecan complexity comes from the fact that it tries to make some routine actions easier by introducing magical constructs such as singleton Request\Response objects and heavy use of decorators. It makes sense for some simple applications built by people new to web development, but it creates additional obstacles when you know what you're doing. This opinionated design along with poor modularity on Pecan side and our initial copycat approach to st2api development lead us to where we are now.
Swagger seems like a decent way of writing REST spec, but server code they generate for python flask is even more magical that we have now (http://editor.swagger.io/, see for yourself) and for now I don't really see how it's going to handle code changes after we decide to alter the spec and regenerate it. We probably should not expect it to solve all of our problems out the box and be ready to the fact that some assembly might be required. |
I went ahead and compared the version we're using with current pecan master (StorminStanley/pecan@st2-patched...pecan:master). Only a couple of things have really changed:
|
It seems to me that in python world, simplerouter is pretty close to a minimum viable implementation of http service. You can probably throw away WebOb too if you fine working with environ directly, though it's likely a level of convenience we want to keep. Redirects may not be all that hugely beneficial to us; we may also want to convert our current hooks to work as wsgi middlewares or integrate them into router pipeline somehow. |
As far as Swagger-compatible frameworks go, connexion seems to be the only option. It pulls Flask with it, although mostly it only uses its Request\Response objects (same thing WebOb provides). It also only works with gevent and not eventlet (as far as I can tell, they are mostly compatible, but the particular degree is something we would need to investigate more thoroughly). |
Since no one is biting, I'll continue in a form of monologue =) Thing is, for all the issues in this thread, only "Nicer and more "RESTful" paths" is a breaking change to the API, the rest is just refactoring that would be good to do either before or during v2 implementation. I'd like us to discuss other types of breaking changes that are desperately needed:
When we're talking about API v2, this are the things I think about in the first place, not whether we're going to use swagger or grpc, Pecan or Flask. Those can be done for v1 providing we have enough motivation... |
I'll close this since a lot of it has been resolved in #2727 and more will be once we introduce new v2 API endpoints without the legacy pecan code and paths. |
Background
From the end-user perspective, API v1 is not too bad (there are definitely worse RESTful APIs out there), but from developer and maintainability perspective, API v1 is quite horrible (it has been causing us a lot of pain).
A lot of that is related to our (ab)use of Pecan. We use Pecan in ways it was never really meant to be used. This results in a lot of hacks (jsexpose is a terrible monstrosity) and hard-to-maintain controller code. In addition to that, using Pecan ties us to a particular not-too-restful URL scheme (in theory, we could actually make it work with any kind of URL scheme we want, but this would require horrible hacks to the code and code would be even harder to follow and maintain).
Goals / requirements
New API should address the following things:
/v1/packs/views/files/<pack name>
vs/v2/packs/<pack name>/files
,/v1/config_schema/<pack name>
vs/v2/packs/<pack name>/config_schema
, etc.)Open questions and Implementation details
There are some open questions and implementation details we need to decide about:
I've use Django, Flask and Tornado for such purposes in the past. Django is obviously not a good fit and over-kill and Tornado requires Tornado-specific async code which is no go for us since we are already bought into the eventlet ecosystem.
Flask might be an OK middle-ground, but I still think it's a slight over-kill and too bloated for simple API services.
I do think we should consider using something like Swagger or grpc. Not matter what kind of approach we go with, having some kind of description / "schema" for API is a good thing. Manually writing low-level API clients and API docs is error prone and a waste of time (it's too easy for things to get out of sync, hard to maintain, etc.).
Using something like Swagger / Thrift / grpc would potentially also allow us to version models and API responses, but I think that's out of scope of this change (doing all of that mentioned above is already quite a lot of work and we can't do and fix everything as part of this change).
I think all of us still need to do some more research, but let's get the debate going. Feedback and comments are welcome.
The text was updated successfully, but these errors were encountered: