| Message ID | 1460744647-9729-9-git-send-email-andy.doan@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On 15 Apr 13:24, Andy Doan wrote: > This allows a user to download the Patch in the MBOX format directly > from the API. > > Security Constraints: > * Read-only to everyone I don't know what the spec says, but I don't think this endpoint is necessary. We already have a way to get diffs and mboxes, so we can just provide links to that in our original 'patch' response. GitHub do something similar with their 'raw_url' parameter for gists [1]. Less endpoint + less code = winning? Stephen [1] https://developer.github.com/v3/gists/ > > Signed-off-by: Andy Doan <andy.doan@linaro.org> > --- > patchwork/tests/test_rest_api.py | 8 ++++++++ > patchwork/views/rest_api.py | 7 +++++++ > 2 files changed, 15 insertions(+) > > diff --git a/patchwork/tests/test_rest_api.py b/patchwork/tests/test_rest_api.py > index f98ad75..fe21fcf 100644 > --- a/patchwork/tests/test_rest_api.py > +++ b/patchwork/tests/test_rest_api.py > @@ -237,6 +237,14 @@ class TestPatchAPI(APITestCase): > self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) > self.assertEqual(1, Patch.objects.all().count()) > > + def test_mbox(self): > + """Ensure we can download the raw mbox version of the patch.""" > + patches = create_patches() > + resp = self.client.get('/api/1.0/patches/%d/mbox/' % patches[0].id) > + self.assertEqual(status.HTTP_200_OK, resp.status_code) > + self.assertIn('\nMIME-Version:', resp.content) > + self.assertIn(patches[0].diff, resp.content) > + > > @unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API') > class TestCheckAPI(APITestCase): > diff --git a/patchwork/views/rest_api.py b/patchwork/views/rest_api.py > index 3918a7a..2e7fcbb 100644 > --- a/patchwork/views/rest_api.py > +++ b/patchwork/views/rest_api.py > @@ -20,6 +20,7 @@ > from django.conf.urls import url, include > > from patchwork.models import Check, Patch, Person, Project > +from patchwork.views.patch import mbox > > from rest_framework import permissions > from rest_framework.exceptions import PermissionDenied > @@ -142,6 +143,11 @@ class CheckViewSet(GenericViewSet): > return Response({'state': state}) > > > +class MboxViewSet(GenericViewSet): > + def list(self, request, patch_pk): > + return mbox(request, patch_pk) > + > + > router = DefaultRouter() > router.register('patches', PatchViewSet, 'patch') > router.register('people', PeopleViewSet, 'person') > @@ -150,6 +156,7 @@ router.register('projects', ProjectViewSet, 'project') > patches_router = NestedSimpleRouter(router, r'patches', lookup='patch') > patches_router.register(r'checks', ChecksViewSet, base_name='patch-checks') > patches_router.register(r'check', CheckViewSet, base_name='patch-check') > +patches_router.register(r'mbox', MboxViewSet, base_name='patch-mbox') > > urlpatterns = [ > url(r'^api/1.0/', include(router.urls)), > -- > 2.7.4 > > _______________________________________________ > Patchwork mailing list > Patchwork@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/patchwork
On 05/09/2016 09:03 AM, Finucane, Stephen wrote: > I don't know what the spec says, but I don't think this endpoint > is necessary. We already have a way to get diffs and mboxes, so we > can just provide links to that in our original 'patch' response. > GitHub do something similar with their 'raw_url' parameter for gists > [1]. Less endpoint + less code = winning? It was in the spec, I'd vote for removing it and providing a URL to raw patch. I'll put that together in the next patchset.
diff --git a/patchwork/tests/test_rest_api.py b/patchwork/tests/test_rest_api.py index f98ad75..fe21fcf 100644 --- a/patchwork/tests/test_rest_api.py +++ b/patchwork/tests/test_rest_api.py @@ -237,6 +237,14 @@ class TestPatchAPI(APITestCase): self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) self.assertEqual(1, Patch.objects.all().count()) + def test_mbox(self): + """Ensure we can download the raw mbox version of the patch.""" + patches = create_patches() + resp = self.client.get('/api/1.0/patches/%d/mbox/' % patches[0].id) + self.assertEqual(status.HTTP_200_OK, resp.status_code) + self.assertIn('\nMIME-Version:', resp.content) + self.assertIn(patches[0].diff, resp.content) + @unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API') class TestCheckAPI(APITestCase): diff --git a/patchwork/views/rest_api.py b/patchwork/views/rest_api.py index 3918a7a..2e7fcbb 100644 --- a/patchwork/views/rest_api.py +++ b/patchwork/views/rest_api.py @@ -20,6 +20,7 @@ from django.conf.urls import url, include from patchwork.models import Check, Patch, Person, Project +from patchwork.views.patch import mbox from rest_framework import permissions from rest_framework.exceptions import PermissionDenied @@ -142,6 +143,11 @@ class CheckViewSet(GenericViewSet): return Response({'state': state}) +class MboxViewSet(GenericViewSet): + def list(self, request, patch_pk): + return mbox(request, patch_pk) + + router = DefaultRouter() router.register('patches', PatchViewSet, 'patch') router.register('people', PeopleViewSet, 'person') @@ -150,6 +156,7 @@ router.register('projects', ProjectViewSet, 'project') patches_router = NestedSimpleRouter(router, r'patches', lookup='patch') patches_router.register(r'checks', ChecksViewSet, base_name='patch-checks') patches_router.register(r'check', CheckViewSet, base_name='patch-check') +patches_router.register(r'mbox', MboxViewSet, base_name='patch-mbox') urlpatterns = [ url(r'^api/1.0/', include(router.urls)),
This allows a user to download the Patch in the MBOX format directly from the API. Security Constraints: * Read-only to everyone Signed-off-by: Andy Doan <andy.doan@linaro.org> --- patchwork/tests/test_rest_api.py | 8 ++++++++ patchwork/views/rest_api.py | 7 +++++++ 2 files changed, 15 insertions(+)