[RFC 1/2] REST: Add base configuration hooks for a REST API

[Finucane, Stephen]

On 24 Mar 13:43, Andy Doan wrote:
> On 03/24/2016 10:36 AM, Finucane, Stephen wrote:
> >On 16 Mar 11:13, Andy Doan wrote:
> >>This adds the ability to expose a REST API based on the Django REST
> >>framework project. Since this project isn't packaged in most current
> >>distributions, we ensure that its both installed and enabled before
> >>trying to use it.
> >
> >Having hacked around with tastypie myself, I think going with DRF is
> >likely the best approach at this point. This series is a good start,
> >comments below withstanding. We should obviously use as much of
> >Damien's work as possible also, rather than reinventing the wheel.
> 
> agreed. My plan was basically to step through his API
> endpoint-by-endpoint to produce a full API. I figured it should be
> one patch series, but that seems the easiest way to make it easy to
> review.

Sounds like a plan for success.

> >However, I would like to scope out the endpoints we're going to expose
> >and, to a lesser degree (because we can fix mistakes with API
> >revisions) the kind of responses we would return. If you agree, then I
> >think we can do this as a formalized document at the beginning, or
> >develop said document iteratively through many small series like this
> >one. I have a draft Swagger/OpenAPI document that I could provide, if
> >this would help. Thoughts?
> 
> Sure - the docs were one thing I was sure how to approach. I'm just
> sort of using Damien's code as my reference. Please share what you
> have. I'd lean towards a hybrid approach for Swagger doc. Maybe we
> create a full drafted spec up front to ensure we have a sensible
> API, and then incrementally update it with each endpoint patch.

Expect this shortly. Swagger gives us some really cool features like,
for example, the ability to autogenerate our clients. Making use of
the Swagger UI would avoid us having to manually write repetitive
rST/Markdown documents. OpenStack are switching to this for all APIs
and I think it's a winning formula.

> >>diff --git a/patchwork/settings/base.py b/patchwork/settings/base.py
> >>index ef2a9ee..7ef6e31 100644
> >>--- a/patchwork/settings/base.py
> >>+++ b/patchwork/settings/base.py
> >>@@ -27,6 +27,14 @@ INSTALLED_APPS = [
> >>      'patchwork',
> >>  ]
> >>
> >>+try:
> >>+    # django rest framework isn't a standard package in most distros, so
> >>+    # don't make it cumpulsory
> >>+    import rest_framework  # NOQA
> >>+    INSTALLED_APPS.append('rest_framework')
> >>+except ImportError:
> >>+    pass
> >>+
> >
> >IMO, if 'ENABLE_REST_API' is 'True' then this should actually raise the
> >error, so check that value instead of using a try..catch? Note: I do
> >realise that placing 'ENABLE_REST_API' higher in the file will mess up
> >the structure of the file, but it seems the lesser of two evils :)
> 
> The problem I was having is that people aren't going to customize
> base.py. So "ENABLE_REST_API" in my case gets enabled something
> like:
> 
>  # my production.py
>  from patchwork.base.settings import *
>  ENABLE_REST_API = True
> 
> So base.py doesn't have the context to make that decision, and I was
> trying to minimize the work someone needs to enable the feature.

Ohhh, yes - that makes sense. Can you note this in the code (with name
attached) for anyone who might have the same question in the future?

> >>diff --git a/patchwork/urls.py b/patchwork/urls.py
> >>index 022b92c..095df75 100644
> >>--- a/patchwork/urls.py
> >>+++ b/patchwork/urls.py
> >>@@ -132,6 +132,15 @@ if settings.ENABLE_XMLRPC:
> >>              name='pwclientrc'),
> >>      ]
> >>
> >>+if settings.ENABLE_REST_API:
> >>+    if 'rest_framework' not in settings.INSTALLED_APPS:
> >>+        raise RuntimeError(
> >>+            'djangorestframework must be installed to enable the REST API.')
> >
> >Doing the above allows you to remove this check.
> 
> This was how I was addressing my issue with base.py/production.py

Understood.