[PATCH v3 05/16] REST: Explicitly define fields

[Stephen Finucane]

On Mon, 2016-11-28 at 15:59 -0600, Andy Doan wrote:
> On 11/25/2016 12:18 PM, Stephen Finucane wrote:
> > Explicitly define included fields (using 'Meta.fields') rather than
> > those that should be excluded (using 'Meta.exclude'). This ensure
> > fields
> > that are exposed by the API don't change unless we explicitly
> > change
> > them
> > 
> > The only side-effect of this change should be a consistent ordering
> > of
> > the output - the ordering used in the 'Meta.fields' will be
> > consistently
> > used, thus ordering is now important and some fields are moved
> > around
> > to reflect this.
> > 
> > Signed-off-by: Stephen Finucane <stephen at that.guru>
> 
> Feel free to add my:
>   Reviewed-by: Andy Doan <andy.doan at linaro.org>
> 
> After answering my question below:
> 
> > diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py
> > index 3f464b2..3af5994 100644
> > --- a/patchwork/api/patch.py
> > +++ b/patchwork/api/patch.py
> > @@ -72,11 +72,12 @@ class
> > PatchSerializer(HyperlinkedModelSerializer):
> >      class Meta:
> >          model = Patch
> >          list_serializer_class = PatchListSerializer
> > +        fields = ('url', 'project', 'msgid', 'date', 'name',
> > 'commit_ref',
> > +                  'pull_url', 'state', 'archived', 'hash',
> > 'submitter',
> > +                  'delegate', 'mbox', 'check', 'checks', 'tags',
> > 'headers',
> > +                  'content', 'diff')
> >          read_only_fields = ('project', 'name', 'date',
> > 'submitter', 'diff',
> >                              'content', 'hash', 'msgid')
> > -        # there's no need to expose an entire "tags" endpoint, so
> > we custom
> > -        # render this field
> > -        exclude = ('tags',)
> 
> You are adding "tags" as a field here. You have a "TODO" in the
> previous 
> patch for "get_tags". Is performance acceptable when including, or
> is 
> there a potential side-effect?

Short answer: this has no effect, good or bad, on performance. Long
answer: from what I've seen, you can include a field in 'fields' and
later override it, resulting in the original serializer (whatever that
may be) never being called. The only purpose of including 'tags' here
is to ensure the output is correctly ordered. The performance still
sucks, but we fix it in a follow up patch so we're fine on that front.

Hope that helps?

Stephen