Page MenuHomeSoftware Heritage

T431: Drop FlaskAPI for custom documentation
ClosedPublic

Authored by jbertran on Aug 17 2016, 11:43 AM.

Details

Summary

drop Flask API for custom documentation

  • apidoc.py: provide decorators for documenting API processing methods
  • api.html: display all API documentation endpoints and associated doc
  • apidoc.html: display documentation on endpoint, and prettified response if an API route was used

browse: Open /api/1/doc/ browsing view
API: document processing methods with new APIdoc

  • Use new APIdoc decorators to document API methods.
  • Docstrings now contain only the function's overview.
Test Plan

Included

Diff Detail

Repository
rDWAPPS Web applications
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jbertran retitled this revision from to T431: Drop FlaskAPI for custom documentation.
jbertran updated this object.
jbertran edited the test plan for this revision. (Show Details)

Note:

  • I'm not entirely satisfied (in general with apidoc) that my way is the right way. Don't hesitate to suggest big changes in the approach for the classes/modules.
  • We are currently missing some formatting power for the documentation strings, that I haven't solved yet since the diff was becoming cumbersome Used docutils to obtain HTML formatting of reST.
  • This makes a lot of code that specifically supported FlaskAPI quirks obsolete, including most of the renderers module (?) and forcing renderer choice in the browse endpoints. This will be a task of its own. Fixed here.
In D98#2452, @jbertran wrote:

Note:

  • I'm not entirely satisfied (in general with apidoc) that my way is the right way. Don't hesitate to suggest big changes in the approach for the classes/modules.

Hey,

I started doing inline reviewing, and then I noticed the following comment

Caution: this MUST be the last decorator in the apidoc decorator stack,
or the decorated endpoint breaks

This really makes me wonder whether the "decorator tower" is the right approach for this problem. I think your currently proposed approach is workable and you should not take this as a blocker for merge, but rather as something that might be improved in the future.

The problem we're trying to solve is documenting, in a DRY and declarative way, API endpoints.

What that means in practice:

  • We have one view for the API endpoint
    • This view can be accessed by one or several routes parameterized with typed arguments
  • We declare the documentation for the API endpoint, its parameters, its return values, using a declarative system
  • We need to generate a "documentation route" allowing access to the documentation for the given API endpoint
  • We collect all the "documentation routes" inside an API index

The current tower of decorators approach is brittle, as it depends on the order of decoration to do its work in the right order.

We might be better off with a class/metaclass approach such as the one used for Django models, or Flask class based views.

That way, we can use the whole class declaration as our "structured documentation data", and run the whole code in one fell swoop without caring about the order of declarations.

swh/web/ui/apidoc.py
17–46 ↗(On Diff #336)

These should probably inherit from enum.Enum

jbertran edited edge metadata.

Move response/mimetype logic + tests to renderers

jbertran edited edge metadata.

renderers: generate HTML from reST text

ardumont added inline comments.
swh/web/ui/renderers.py
12 ↗(On Diff #342)

This should probably been cleaned up.
Then, every subsequent classes that depended on it in this module should go as well.

Note that this is not a blocker either.

13 ↗(On Diff #342)

Same here.

jbertran edited edge metadata.
  • Apidoc: switch to Enum classes for doc constants
  • Renderers: remove FlaskAPI rendering logic
ardumont added a reviewer: ardumont.

Seems good to go.

Packaging wise, there remains the requirements.txt and debian/control to update by removing the dependency on flask-api.
This can be done later ^^

This revision is now accepted and ready to land.Aug 19 2016, 1:42 PM
This revision was automatically updated to reflect the committed changes.