diff mbox series

[01/17] REST: Add additional documentation

Message ID 20181030111916.7342-2-stephen@that.guru
State Accepted
Headers show
Series Add OpenAPI 3.0.0 REST API spec | expand

Commit Message

Stephen Finucane Oct. 30, 2018, 11:19 a.m. UTC
As noted in the Django REST Framework docs [1], views that support
multiple methods can and should split their documentation using
'method:' style delimiters. Do just this.

[1] https://www.django-rest-framework.org/topics/documenting-your-api/#documenting-your-views

Signed-off-by: Stephen Finucane <stephen@that.guru>
---
 patchwork/api/check.py   |  8 +++++++-
 patchwork/api/index.py   |  1 +
 patchwork/api/patch.py   | 10 +++++++++-
 patchwork/api/project.py | 11 ++++++++++-
 patchwork/api/user.py    | 11 ++++++++++-
 5 files changed, 37 insertions(+), 4 deletions(-)

Comments

Daniel Axtens Nov. 8, 2018, 1:15 p.m. UTC | #1
Stephen Finucane <stephen@that.guru> writes:

> As noted in the Django REST Framework docs [1], views that support
> multiple methods can and should split their documentation using
> 'method:' style delimiters. Do just this.
>
> [1] https://www.django-rest-framework.org/topics/documenting-your-api/#documenting-your-views
>
> Signed-off-by: Stephen Finucane <stephen@that.guru>
> ---
>  patchwork/api/check.py   |  8 +++++++-
>  patchwork/api/index.py   |  1 +
>  patchwork/api/patch.py   | 10 +++++++++-
>  patchwork/api/project.py | 11 ++++++++++-
>  patchwork/api/user.py    | 11 ++++++++++-
>  5 files changed, 37 insertions(+), 4 deletions(-)
>
> diff --git a/patchwork/api/check.py b/patchwork/api/check.py
> index 48019e72..4771455f 100644
> --- a/patchwork/api/check.py
> +++ b/patchwork/api/check.py
> @@ -74,7 +74,13 @@ class CheckMixin(object):
>  
>  
>  class CheckListCreate(CheckMixin, ListCreateAPIView):
> -    """List or create checks."""
> +    """
> +    get:
> +    List checks.
> +
> +    post:
> +    Create a check.
> +    """
>  
>      lookup_url_kwarg = 'patch_id'
>      ordering = 'id'
> diff --git a/patchwork/api/index.py b/patchwork/api/index.py
> index 2266635c..45485c91 100644
> --- a/patchwork/api/index.py
> +++ b/patchwork/api/index.py
> @@ -11,6 +11,7 @@ from rest_framework.views import APIView
>  class IndexView(APIView):
>  
>      def get(self, request, *args, **kwargs):
> +        """List API resources."""
>          return Response({
>              'projects': reverse('api-project-list', request=request),
>              'users': reverse('api-user-list', request=request),
> diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py
> index 6367dd5d..523378da 100644
> --- a/patchwork/api/patch.py
> +++ b/patchwork/api/patch.py
> @@ -188,8 +188,16 @@ class PatchList(ListAPIView):
>  
>  
>  class PatchDetail(RetrieveUpdateAPIView):
> -    """Show a patch."""
> +    """
> +    get:
> +    Show a patch.
> +
> +    patch:
> +    Update a patch.
>  
> +    put:
> +    Update a patch.
I thought put created a patch/project/person. Does it also serve to
update one? I know we got an issue via the mailing list about put vs
update; should the docs make the distinction a bit clearer?

Apart from that, LGTM.

Regards,
Daniel

> +    """
>      permission_classes = (PatchworkPermission,)
>      serializer_class = PatchDetailSerializer
>  
> diff --git a/patchwork/api/project.py b/patchwork/api/project.py
> index 22fb1c4e..d7bb1f21 100644
> --- a/patchwork/api/project.py
> +++ b/patchwork/api/project.py
> @@ -74,6 +74,15 @@ class ProjectList(ProjectMixin, ListAPIView):
>  
>  
>  class ProjectDetail(ProjectMixin, RetrieveUpdateAPIView):
> -    """Show a project."""
> +    """
> +    get:
> +    Show a project.
> +
> +    patch:
> +    Update a project.
> +
> +    put:
> +    Update a project.
> +    """
>  
>      pass
> diff --git a/patchwork/api/user.py b/patchwork/api/user.py
> index e11bed4b..29944e22 100644
> --- a/patchwork/api/user.py
> +++ b/patchwork/api/user.py
> @@ -48,6 +48,15 @@ class UserList(UserMixin, ListAPIView):
>  
>  
>  class UserDetail(UserMixin, RetrieveUpdateAPIView):
> -    """Show a user."""
> +    """
> +    get:
> +    Show a user.
> +
> +    patch:
> +    Update a user.
> +
> +    put:
> +    Update a user.
> +    """
>  
>      pass
> -- 
> 2.17.2
>
> _______________________________________________
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
Stephen Finucane Nov. 8, 2018, 2:02 p.m. UTC | #2
On Fri, 2018-11-09 at 00:15 +1100, Daniel Axtens wrote:
> Stephen Finucane <stephen@that.guru> writes:
> 
> > As noted in the Django REST Framework docs [1], views that support
> > multiple methods can and should split their documentation using
> > 'method:' style delimiters. Do just this.
> > 
> > [1] https://www.django-rest-framework.org/topics/documenting-your-api/#documenting-your-views
> > 
> > Signed-off-by: Stephen Finucane <stephen@that.guru>
> > ---
> >  patchwork/api/check.py   |  8 +++++++-
> >  patchwork/api/index.py   |  1 +
> >  patchwork/api/patch.py   | 10 +++++++++-
> >  patchwork/api/project.py | 11 ++++++++++-
> >  patchwork/api/user.py    | 11 ++++++++++-
> >  5 files changed, 37 insertions(+), 4 deletions(-)
> > 
> > diff --git a/patchwork/api/check.py b/patchwork/api/check.py
> > index 48019e72..4771455f 100644
> > --- a/patchwork/api/check.py
> > +++ b/patchwork/api/check.py
> > @@ -74,7 +74,13 @@ class CheckMixin(object):
> >  
> >  
> >  class CheckListCreate(CheckMixin, ListCreateAPIView):
> > -    """List or create checks."""
> > +    """
> > +    get:
> > +    List checks.
> > +
> > +    post:
> > +    Create a check.
> > +    """
> >  
> >      lookup_url_kwarg = 'patch_id'
> >      ordering = 'id'
> > diff --git a/patchwork/api/index.py b/patchwork/api/index.py
> > index 2266635c..45485c91 100644
> > --- a/patchwork/api/index.py
> > +++ b/patchwork/api/index.py
> > @@ -11,6 +11,7 @@ from rest_framework.views import APIView
> >  class IndexView(APIView):
> >  
> >      def get(self, request, *args, **kwargs):
> > +        """List API resources."""
> >          return Response({
> >              'projects': reverse('api-project-list', request=request),
> >              'users': reverse('api-user-list', request=request),
> > diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py
> > index 6367dd5d..523378da 100644
> > --- a/patchwork/api/patch.py
> > +++ b/patchwork/api/patch.py
> > @@ -188,8 +188,16 @@ class PatchList(ListAPIView):
> >  
> >  
> >  class PatchDetail(RetrieveUpdateAPIView):
> > -    """Show a patch."""
> > +    """
> > +    get:
> > +    Show a patch.
> > +
> > +    patch:
> > +    Update a patch.
> >  
> > +    put:
> > +    Update a patch.
> I thought put created a patch/project/person. Does it also serve to
> update one? I know we got an issue via the mailing list about put vs
> update; should the docs make the distinction a bit clearer?

This is how django-rest-framework and most other APIs think about these
verbs.

GET
   Get an existing resource.

DELETE
   Delete an existing resource.

POST
   Create a new resource.

PUT
   Update an existing resource by sending the entire resource with
   updated values.

PATCH
   Update an existing resource by sending just the updated fields [*].

So saying that PUT updates a patch is correct.

Stephen

[*] Technically PATCH is supposed to contain instructions on updating
the resource, rather than just the updated fields, as noted in the
below article. However, I've never seen this done in practice and as
that article now notes, a new RFC - RFC 7396 - describes how people
*actually* do this stuff.

  https://williamdurand.fr/2014/02/14/please-do-not-patch-like-an-idiot/


> Apart from that, LGTM.
> 
> Regards,
> Daniel
> 
> > +    """
> >      permission_classes = (PatchworkPermission,)
> >      serializer_class = PatchDetailSerializer
> >  
> > diff --git a/patchwork/api/project.py b/patchwork/api/project.py
> > index 22fb1c4e..d7bb1f21 100644
> > --- a/patchwork/api/project.py
> > +++ b/patchwork/api/project.py
> > @@ -74,6 +74,15 @@ class ProjectList(ProjectMixin, ListAPIView):
> >  
> >  
> >  class ProjectDetail(ProjectMixin, RetrieveUpdateAPIView):
> > -    """Show a project."""
> > +    """
> > +    get:
> > +    Show a project.
> > +
> > +    patch:
> > +    Update a project.
> > +
> > +    put:
> > +    Update a project.
> > +    """
> >  
> >      pass
> > diff --git a/patchwork/api/user.py b/patchwork/api/user.py
> > index e11bed4b..29944e22 100644
> > --- a/patchwork/api/user.py
> > +++ b/patchwork/api/user.py
> > @@ -48,6 +48,15 @@ class UserList(UserMixin, ListAPIView):
> >  
> >  
> >  class UserDetail(UserMixin, RetrieveUpdateAPIView):
> > -    """Show a user."""
> > +    """
> > +    get:
> > +    Show a user.
> > +
> > +    patch:
> > +    Update a user.
> > +
> > +    put:
> > +    Update a user.
> > +    """
> >  
> >      pass
> > -- 
> > 2.17.2
> > 
> > _______________________________________________
> > Patchwork mailing list
> > Patchwork@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/patchwork
Daniel Axtens Nov. 10, 2018, 11:27 p.m. UTC | #3
Stephen Finucane <stephen@that.guru> writes:

> On Fri, 2018-11-09 at 00:15 +1100, Daniel Axtens wrote:
>> Stephen Finucane <stephen@that.guru> writes:
>> 
>> > As noted in the Django REST Framework docs [1], views that support
>> > multiple methods can and should split their documentation using
>> > 'method:' style delimiters. Do just this.
>> > 
>> > [1] https://www.django-rest-framework.org/topics/documenting-your-api/#documenting-your-views
>> > 
>> > Signed-off-by: Stephen Finucane <stephen@that.guru>
>> > ---
>> >  patchwork/api/check.py   |  8 +++++++-
>> >  patchwork/api/index.py   |  1 +
>> >  patchwork/api/patch.py   | 10 +++++++++-
>> >  patchwork/api/project.py | 11 ++++++++++-
>> >  patchwork/api/user.py    | 11 ++++++++++-
>> >  5 files changed, 37 insertions(+), 4 deletions(-)
>> > 
>> > diff --git a/patchwork/api/check.py b/patchwork/api/check.py
>> > index 48019e72..4771455f 100644
>> > --- a/patchwork/api/check.py
>> > +++ b/patchwork/api/check.py
>> > @@ -74,7 +74,13 @@ class CheckMixin(object):
>> >  
>> >  
>> >  class CheckListCreate(CheckMixin, ListCreateAPIView):
>> > -    """List or create checks."""
>> > +    """
>> > +    get:
>> > +    List checks.
>> > +
>> > +    post:
>> > +    Create a check.
>> > +    """
>> >  
>> >      lookup_url_kwarg = 'patch_id'
>> >      ordering = 'id'
>> > diff --git a/patchwork/api/index.py b/patchwork/api/index.py
>> > index 2266635c..45485c91 100644
>> > --- a/patchwork/api/index.py
>> > +++ b/patchwork/api/index.py
>> > @@ -11,6 +11,7 @@ from rest_framework.views import APIView
>> >  class IndexView(APIView):
>> >  
>> >      def get(self, request, *args, **kwargs):
>> > +        """List API resources."""
>> >          return Response({
>> >              'projects': reverse('api-project-list', request=request),
>> >              'users': reverse('api-user-list', request=request),
>> > diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py
>> > index 6367dd5d..523378da 100644
>> > --- a/patchwork/api/patch.py
>> > +++ b/patchwork/api/patch.py
>> > @@ -188,8 +188,16 @@ class PatchList(ListAPIView):
>> >  
>> >  
>> >  class PatchDetail(RetrieveUpdateAPIView):
>> > -    """Show a patch."""
>> > +    """
>> > +    get:
>> > +    Show a patch.
>> > +
>> > +    patch:
>> > +    Update a patch.
>> >  
>> > +    put:
>> > +    Update a patch.
>> I thought put created a patch/project/person. Does it also serve to
>> update one? I know we got an issue via the mailing list about put vs
>> update; should the docs make the distinction a bit clearer?
>
> This is how django-rest-framework and most other APIs think about these
> verbs.
>
> GET
>    Get an existing resource.
>
> DELETE
>    Delete an existing resource.
>
> POST
>    Create a new resource.
>
> PUT
>    Update an existing resource by sending the entire resource with
>    updated values.
>
> PATCH
>    Update an existing resource by sending just the updated fields [*].
>
> So saying that PUT updates a patch is correct.
>

Ok, thanks for that.

Regards,
Daniel

> Stephen
>
> [*] Technically PATCH is supposed to contain instructions on updating
> the resource, rather than just the updated fields, as noted in the
> below article. However, I've never seen this done in practice and as
> that article now notes, a new RFC - RFC 7396 - describes how people
> *actually* do this stuff.
>
>   https://williamdurand.fr/2014/02/14/please-do-not-patch-like-an-idiot/
>
>
>> Apart from that, LGTM.
>> 
>> Regards,
>> Daniel
>> 
>> > +    """
>> >      permission_classes = (PatchworkPermission,)
>> >      serializer_class = PatchDetailSerializer
>> >  
>> > diff --git a/patchwork/api/project.py b/patchwork/api/project.py
>> > index 22fb1c4e..d7bb1f21 100644
>> > --- a/patchwork/api/project.py
>> > +++ b/patchwork/api/project.py
>> > @@ -74,6 +74,15 @@ class ProjectList(ProjectMixin, ListAPIView):
>> >  
>> >  
>> >  class ProjectDetail(ProjectMixin, RetrieveUpdateAPIView):
>> > -    """Show a project."""
>> > +    """
>> > +    get:
>> > +    Show a project.
>> > +
>> > +    patch:
>> > +    Update a project.
>> > +
>> > +    put:
>> > +    Update a project.
>> > +    """
>> >  
>> >      pass
>> > diff --git a/patchwork/api/user.py b/patchwork/api/user.py
>> > index e11bed4b..29944e22 100644
>> > --- a/patchwork/api/user.py
>> > +++ b/patchwork/api/user.py
>> > @@ -48,6 +48,15 @@ class UserList(UserMixin, ListAPIView):
>> >  
>> >  
>> >  class UserDetail(UserMixin, RetrieveUpdateAPIView):
>> > -    """Show a user."""
>> > +    """
>> > +    get:
>> > +    Show a user.
>> > +
>> > +    patch:
>> > +    Update a user.
>> > +
>> > +    put:
>> > +    Update a user.
>> > +    """
>> >  
>> >      pass
>> > -- 
>> > 2.17.2
>> > 
>> > _______________________________________________
>> > Patchwork mailing list
>> > Patchwork@lists.ozlabs.org
>> > https://lists.ozlabs.org/listinfo/patchwork
diff mbox series

Patch

diff --git a/patchwork/api/check.py b/patchwork/api/check.py
index 48019e72..4771455f 100644
--- a/patchwork/api/check.py
+++ b/patchwork/api/check.py
@@ -74,7 +74,13 @@  class CheckMixin(object):
 
 
 class CheckListCreate(CheckMixin, ListCreateAPIView):
-    """List or create checks."""
+    """
+    get:
+    List checks.
+
+    post:
+    Create a check.
+    """
 
     lookup_url_kwarg = 'patch_id'
     ordering = 'id'
diff --git a/patchwork/api/index.py b/patchwork/api/index.py
index 2266635c..45485c91 100644
--- a/patchwork/api/index.py
+++ b/patchwork/api/index.py
@@ -11,6 +11,7 @@  from rest_framework.views import APIView
 class IndexView(APIView):
 
     def get(self, request, *args, **kwargs):
+        """List API resources."""
         return Response({
             'projects': reverse('api-project-list', request=request),
             'users': reverse('api-user-list', request=request),
diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py
index 6367dd5d..523378da 100644
--- a/patchwork/api/patch.py
+++ b/patchwork/api/patch.py
@@ -188,8 +188,16 @@  class PatchList(ListAPIView):
 
 
 class PatchDetail(RetrieveUpdateAPIView):
-    """Show a patch."""
+    """
+    get:
+    Show a patch.
+
+    patch:
+    Update a patch.
 
+    put:
+    Update a patch.
+    """
     permission_classes = (PatchworkPermission,)
     serializer_class = PatchDetailSerializer
 
diff --git a/patchwork/api/project.py b/patchwork/api/project.py
index 22fb1c4e..d7bb1f21 100644
--- a/patchwork/api/project.py
+++ b/patchwork/api/project.py
@@ -74,6 +74,15 @@  class ProjectList(ProjectMixin, ListAPIView):
 
 
 class ProjectDetail(ProjectMixin, RetrieveUpdateAPIView):
-    """Show a project."""
+    """
+    get:
+    Show a project.
+
+    patch:
+    Update a project.
+
+    put:
+    Update a project.
+    """
 
     pass
diff --git a/patchwork/api/user.py b/patchwork/api/user.py
index e11bed4b..29944e22 100644
--- a/patchwork/api/user.py
+++ b/patchwork/api/user.py
@@ -48,6 +48,15 @@  class UserList(UserMixin, ListAPIView):
 
 
 class UserDetail(UserMixin, RetrieveUpdateAPIView):
-    """Show a user."""
+    """
+    get:
+    Show a user.
+
+    patch:
+    Update a user.
+
+    put:
+    Update a user.
+    """
 
     pass