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

Finucane, Stephen stephen.finucane at intel.com
Mon May 9 23:15:31 AEST 2016


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


More information about the Patchwork mailing list