[PATCHv2 05/10] REST: Add Patches to the API
stephen.finucane at intel.com
Tue May 17 23:25:51 AEST 2016
On 17 May 14:08, Finucane, Stephen wrote:
> On 10 May 17:39, Andy Doan wrote:
> > This exports patches via the REST API.
> supernit: s/exports/exposes :)
> > Security Constraints:
> > * Anyone (logged in or not) can read all objects.
> > * No one can create/delete objects.
> > * Project maintainers are allowed to update (ie "patch"
> > attributes)
> > NOTE: Patch.save was overridden incorrectly and had to be
> > fixed to work with DRF.
> On further inspection, I think the responses still need a little work.
> Sorry for not spotting this the first go round.
> > Signed-off-by: Andy Doan <andy.doan at linaro.org>
> > ---
> > patchwork/models.py | 4 +-
> > patchwork/rest_serializers.py | 9 +++-
> > patchwork/tests/test_rest_api.py | 110 ++++++++++++++++++++++++++++++++++++++-
> > patchwork/views/rest_api.py | 9 +++-
> > 4 files changed, 126 insertions(+), 6 deletions(-)
> > diff --git a/patchwork/models.py b/patchwork/models.py
> > index 4d454c7..6324273 100644
> > --- a/patchwork/models.py
> > +++ b/patchwork/models.py
> > @@ -359,14 +359,14 @@ class Patch(Submission):
> > for tag in tags:
> > self._set_tag(tag, counter[tag])
> > - def save(self):
> > + def save(self, **kwargs):
> > if not hasattr(self, 'state') or not self.state:
> > self.state = get_default_initial_patch_state()
> > if self.hash is None and self.diff is not None:
> > self.hash = hash_patch(self.diff).hexdigest()
> > - super(Patch, self).save()
> > + super(Patch, self).save(**kwargs)
> > self.refresh_tag_counts()
> > diff --git a/patchwork/rest_serializers.py b/patchwork/rest_serializers.py
> > index 60633b4..52a0306 100644
> > --- a/patchwork/rest_serializers.py
> > +++ b/patchwork/rest_serializers.py
> > @@ -17,7 +17,7 @@
> > # along with Patchwork; if not, write to the Free Software
> > # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> > -from patchwork.models import Person, Project
> > +from patchwork.models import Patch, Person, Project
> > from rest_framework.serializers import ModelSerializer
> > @@ -30,3 +30,10 @@ class PersonSerializer(ModelSerializer):
> > class ProjectSerializer(ModelSerializer):
> > class Meta:
> > model = Project
> > +
> > +
> > +class PatchSerializer(ModelSerializer):
> > + class Meta:
> > + model = Patch
> > + read_only_fields = ('project', 'name', 'date', 'submitter', 'diff',
> > + 'content', 'hash', 'msgid')
> So I played around with think the responses could do with a little more
> work. Firstly, here's a sample response (with the really long fields
> "id": 1,
> "mbox_url": "/patch/1/mbox/",
> "msgid": "<1391726961-32072-1-git-send-email-markus.mayer at linaro.org>",
> "date": "2014-02-06T22:49:21",
> "headers": "SNIPPED!",
> "content": "SNIPPED!",
> "name": "[RFC] parsemail.py: Don't search for patches in replies",
> "diff": "SNIPPED!",
> "commit_ref": null,
> "pull_url": null,
> "archived": false,
> "hash": "312958438b253a6436397bb06816c09616d0833e",
> "submitter": 1,
> "project": 1,
> "delegate": null,
> "state": "New",
> "tags": 
> (1) We should have two serializers: one for 'list' and one for 'detail'
> (or 'get'). I would exclude 'headers', 'content' and 'diff' from the
> 'list' response (and test this).
> (2) We should use links for the 'project'/'submitter' fields, rather
> than IDs. Alternatively, we could have a 'project_id' and
> 'project_url' field instead of simply 'project'. What do you prefer?
> (3) Should 'header' be a list, like 'tags'?
> (4) Could/should 'mbox_url' use an absolute link instead of the
> relative one?
(5) Use 'prefetch_related' on 'patch.tags', in addition to
'patch.checks'. There is a cascade of queries to the 'patchwork_tag'
field when accessing '/api/1.0/patches/' at the moment. In general,
we could probably do with making better use of 'defer',
'prefetch_related' and 'select_related'. The existing views code 
would be a good place to start here.
> I'm open to feedback on all of these, of course :) Also, apologies if
> the spec misguided you: I'll do some work on this over the weekend.
> Patchwork mailing list
> Patchwork at lists.ozlabs.org
More information about the Patchwork