[RFC 02/11] REST: Add base configuration hooks for a REST API

[Finucane, Stephen]

On 09 May 14:15, Finucane, Stephen wrote:
> On 15 Apr 13:23, 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.
> > 
> > Signed-off-by: Andy Doan <andy.doan at linaro.org>
> > Inspired-by: Damien Lespiau <damien.lespiau at intel.com>
> 
> Finally getting around to reviewing this. Sorry for the wait - I've
> been very busy :S
> 
> I've a few nits below, but nothing the v2 should be mergeable.
> 
> Stephen
> 
> > ---
> >  patchwork/settings/base.py  | 11 +++++++++++
> >  patchwork/urls.py           |  7 +++++++
> >  patchwork/views/rest_api.py | 28 ++++++++++++++++++++++++++++
> >  requirements-test.txt       |  1 +
> >  4 files changed, 47 insertions(+)
> >  create mode 100644 patchwork/views/rest_api.py
> > 
> > diff --git a/patchwork/settings/base.py b/patchwork/settings/base.py
> > index 2f81d4b..909ae4b 100644
> > --- a/patchwork/settings/base.py
> > +++ b/patchwork/settings/base.py
> > @@ -28,6 +28,14 @@ INSTALLED_APPS = [
> >      'patchwork',
> >  ]
> >  
> > +try:
> > +    # django rest framework isn't a standard package in most distros, so
> > +    # don't make it cumpulsory
> 
> s/cumpulsory/compulsory
> 
> > +    import rest_framework  # NOQA
> > +    INSTALLED_APPS.append('rest_framework')
> 
> nit: The rest of the file uses arithmetic notation, i.e.
> "+= ['rest_framework']". Using '.append' is techincally faster, but
> unless we change every other entry then I would favour consistency.
> 
> > +except ImportError:
> > +    pass
> > +
> >  # HTTP
> >  
> >  MIDDLEWARE_CLASSES = [
> > @@ -148,6 +156,9 @@ NOTIFICATION_FROM_EMAIL = DEFAULT_FROM_EMAIL
> >  # Set to True to enable the Patchwork XML-RPC interface
> >  ENABLE_XMLRPC = False
> >  
> > +# Set to True to enable the Patchwork REST API
> > +ENABLE_REST_API = False
> > +

Also, can you include enable this by default in 'setttings/dev.py'?

> >  # Set to True to enable redirections or URLs from previous versions
> >  # of patchwork
> >  COMPAT_REDIR = True
> > diff --git a/patchwork/urls.py b/patchwork/urls.py
> > index f3fdc5b..c5cabe5 100644
> > --- a/patchwork/urls.py
> > +++ b/patchwork/urls.py
> > @@ -138,6 +138,13 @@ 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.')
> 
> nit: Rather than raising a runtime error, why not just let the
> ImportError bubble up?
> 
> > +    import patchwork.views.rest_api
> > +    urlpatterns += patchwork.views.rest_api.urlpatterns
> > +
> >  # redirect from old urls
> >  if settings.COMPAT_REDIR:
> >      urlpatterns += [
> > diff --git a/patchwork/views/rest_api.py b/patchwork/views/rest_api.py
> > new file mode 100644
> > index 0000000..e451770
> > --- /dev/null
> > +++ b/patchwork/views/rest_api.py
> > @@ -0,0 +1,28 @@
> > +# Patchwork - automated patch tracking system
> > +# Copyright (C) 2016 Linaro Corporation
> > +#
> > +# This file is part of the Patchwork package.
> > +#
> > +# Patchwork is free software; you can redistribute it and/or modify
> > +# it under the terms of the GNU General Public License as published by
> > +# the Free Software Foundation; either version 2 of the License, or
> > +# (at your option) any later version.
> > +#
> > +# Patchwork is distributed in the hope that it will be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +# GNU General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with Patchwork; if not, write to the Free Software
> > +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> > +
> > +from django.conf.urls import url, include
> > +
> > +from rest_framework import routers
> > +
> > +router = routers.DefaultRouter()
> > +
> > +urlpatterns = [
> > +    url(r'^api/1.0/', include(router.urls)),
> > +]
> > diff --git a/requirements-test.txt b/requirements-test.txt
> > index 2d0b32c..fb6a8c8 100644
> > --- a/requirements-test.txt
> > +++ b/requirements-test.txt
> > @@ -2,3 +2,4 @@ mysqlclient==1.3.7  # replace this with psycopg2 for a PostgreSQL backend
> >  django-debug-toolbar==1.4
> >  python-dateutil>2.0,<3.0
> >  selenium>2.0,<3.0
> > +djangorestframework==3.3.3
> 
> This can be a pain to keep current. How about we trust that DRF are
> doing their versioning correctly and limit to >=3.3,<3.4? I think
> I changed this for the other dependendencies recently enough.
> 
> Stephen
> _______________________________________________
> Patchwork mailing list
> Patchwork at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork