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. > > Stephen > > > Signed-off-by: Andy Doan <andy.d...@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 > "SNIPPED!"). > > { > "id": 1, > "mbox_url": "/patch/1/mbox/", > "msgid": > "<1391726961-32072-1-git-send-email-markus.ma...@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 [1] would be a good place to start here. Stephen [1] https://github.com/getpatchwork/patchwork/blob/8aae263/patchwork/views/__init__.py#L289 > 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@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/patchwork _______________________________________________ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork