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

[Andy Doan]

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.

> 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.

>> 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.

>> 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