| Message ID | 1479431925-24865-1-git-send-email-stephen@that.guru |
|---|---|
| State | Superseded |
| Headers | show |
On Fri, 2016-11-18 at 01:18 +0000, Stephen Finucane wrote: > This mostly works out of the box, thanks to Django REST Framework. > Mostly unique fields, like name or email, are excluded as these will be > handled separately. > > Signed-off-by: Stephen Finucane <stephen@that.guru> > --- Hi, Do you know if there's a way to get a reverse ordering of entries in the REST API? For example in the patches API each "page" has 30 entries, and you can get the latest page with ?page=last - but this isn't as nice as a reverse ordering of ids or dates. Do you know if there's a way to do this at present, or is that something that could be made available with filters? - Russell
On 18/11/16 12:18, Stephen Finucane wrote: > This mostly works out of the box, thanks to Django REST Framework. > Mostly unique fields, like name or email, are excluded as these will be > handled separately. > > Signed-off-by: Stephen Finucane <stephen@that.guru> Whoops, accidentally sent my last reply to the RFC version of this patch. Same comments apply here though.
On 21/11/16 11:39, Russell Currey wrote: > Do you know if there's a way to get a reverse ordering of entries in the REST > API? For example in the patches API each "page" has 30 entries, and you can get > the latest page with ?page=last - but this isn't as nice as a reverse ordering > of ids or dates. Do you know if there's a way to do this at present, or is that > something that could be made available with filters? I'd be kinda tempted to consider using cursor pagination for this, which Django REST Framework supports. That's not a filter-related thing though.
On 21/11/16 14:17, Andrew Donnellan wrote: > On 21/11/16 11:39, Russell Currey wrote: >> Do you know if there's a way to get a reverse ordering of entries in >> the REST >> API? For example in the patches API each "page" has 30 entries, and >> you can get >> the latest page with ?page=last - but this isn't as nice as a reverse >> ordering >> of ids or dates. Do you know if there's a way to do this at present, >> or is that >> something that could be made available with filters? > > I'd be kinda tempted to consider using cursor pagination for this, which > Django REST Framework supports. > > That's not a filter-related thing though. Stephen, any thoughts on this?
On Mon, 2016-12-12 at 18:14 +1100, Andrew Donnellan wrote: > On 21/11/16 14:17, Andrew Donnellan wrote: > > On 21/11/16 11:39, Russell Currey wrote: > > > Do you know if there's a way to get a reverse ordering of entries > > > in > > > the REST > > > API? For example in the patches API each "page" has 30 entries, > > > and > > > you can get > > > the latest page with ?page=last - but this isn't as nice as a > > > reverse > > > ordering > > > of ids or dates. Do you know if there's a way to do this at > > > present, > > > or is that > > > something that could be made available with filters? > > > > I'd be kinda tempted to consider using cursor pagination for this, > > which > > Django REST Framework supports. > > > > That's not a filter-related thing though. > > Stephen, any thoughts on this? I don't think we support this yet but it would be easily added. I'm not sure how well cursor pagination would map to of our endpoints, seeing as we use simple, integer-based indexing for all ID fields. My personal preference would be a 'direction=(asc|desc)' parameter, assuming we don't really need to sort on different fields. If we did need to sort then I'd opt for a combination of a 'sort=(date|name|submitter)' parameter (for '/patches' - different values for other endpoints) and 'direction', but I'd also be fine with just 'sort' and a '-' prefix to reverse things. Happy to take patches for any solution :) While we're on the matter, would it be possible to take a look at the patches in the 'series-api' work? There are a couple that still need at least one reviewer before I'd be happy letting them in: https://patchwork.ozlabs.org/bundle/stephenfin/series-api/ Stephen
On 12/12/16 23:34, Stephen Finucane wrote: > I don't think we support this yet but it would be easily added. I'm not > sure how well cursor pagination would map to of our endpoints, seeing > as we use simple, integer-based indexing for all ID fields. My personal > preference would be a 'direction=(asc|desc)' parameter, assuming we > don't really need to sort on different fields. If we did need to sort > then I'd opt for a combination of a 'sort=(date|name|submitter)' > parameter (for '/patches' - different values for other endpoints) and > 'direction', but I'd also be fine with just 'sort' and a '-' prefix to > reverse things. Happy to take patches for any solution :) > > While we're on the matter, would it be possible to take a look at the > patches in the 'series-api' work? There are a couple that still need at > least one reviewer before I'd be happy letting them in: > > https://patchwork.ozlabs.org/bundle/stephenfin/series-api/ We'll need to use something other than page-number based pagination, but something based on the default LimitOffsetPagination class might be good enough for our purposes? Alas I didn't get to the series API patches before you merged them, sorry! Am reviewing/testing v3 of the filter series currently.
On Fri, 2017-01-06 at 18:19 +1100, Andrew Donnellan wrote: > On 12/12/16 23:34, Stephen Finucane wrote: > > I don't think we support this yet but it would be easily added. I'm > > not > > sure how well cursor pagination would map to of our endpoints, > > seeing > > as we use simple, integer-based indexing for all ID fields. My > > personal > > preference would be a 'direction=(asc|desc)' parameter, assuming we > > don't really need to sort on different fields. If we did need to > > sort > > then I'd opt for a combination of a 'sort=(date|name|submitter)' > > parameter (for '/patches' - different values for other endpoints) > > and > > 'direction', but I'd also be fine with just 'sort' and a '-' prefix > > to > > reverse things. Happy to take patches for any solution :) > > > > While we're on the matter, would it be possible to take a look at > > the > > patches in the 'series-api' work? There are a couple that still > > need at > > least one reviewer before I'd be happy letting them in: > > > > https://patchwork.ozlabs.org/bundle/stephenfin/series-api/ > > We'll need to use something other than page-number based pagination, > but > something based on the default LimitOffsetPagination class might be > good > enough for our purposes? Hmm, I don't see a practical difference between LimitOffsetPagination and PageNumberPagination. For example, aren't these two the same thing? ?page=2&per_page=100 ?offset=100&limit=100 and I'd also guess that both would be "reversed" the same way, once [1] is merged: ?page=2&per_page=100&order=-id ?offset=100&limit=100&order=-id Perhaps I've missed some subtle (or unsubtle :)) difference? Would you need more than this? [1] https://patchwork.ozlabs.org/patch/710290/ > Alas I didn't get to the series API patches before you merged them, > sorry! Am reviewing/testing v3 of the filter series currently. No problem - I would have held off for more reviews but I'm supposed to be presenting on Patchwork 2.0 at FOSDEM in four weeks :) Pedal to the metal, to say the least. Stephen
diff --git a/patchwork/api/check.py b/patchwork/api/check.py index 43463fe..9a5c22a 100644 --- a/patchwork/api/check.py +++ b/patchwork/api/check.py @@ -26,6 +26,7 @@ from rest_framework.serializers import HiddenField from rest_framework.serializers import HyperlinkedModelSerializer from rest_framework.serializers import HyperlinkedIdentityField +from patchwork.api.filters import CheckFilter from patchwork.api import MultipleFieldLookupMixin from patchwork.models import Check from patchwork.models import Patch @@ -89,6 +90,7 @@ class CheckMixin(object): queryset = Check.objects.prefetch_related('patch', 'user') serializer_class = CheckSerializer + filter_class = CheckFilter class CheckListCreate(CheckMixin, ListCreateAPIView): diff --git a/patchwork/api/cover.py b/patchwork/api/cover.py index 2674be3..153176c 100644 --- a/patchwork/api/cover.py +++ b/patchwork/api/cover.py @@ -25,6 +25,7 @@ from rest_framework.serializers import HyperlinkedModelSerializer from rest_framework.serializers import HyperlinkedRelatedField from rest_framework.serializers import SerializerMethodField +from patchwork.api.filters import CoverLetterFilter from patchwork.models import CoverLetter @@ -67,6 +68,7 @@ class CoverLetterList(ListAPIView): queryset = CoverLetter.objects.all().prefetch_related( 'series').select_related('submitter').defer('content', 'headers') serializer_class = CoverLetterListSerializer + filter_class = CoverLetterFilter class CoverLetterDetail(RetrieveAPIView): diff --git a/patchwork/api/filters.py b/patchwork/api/filters.py new file mode 100644 index 0000000..a07f464 --- /dev/null +++ b/patchwork/api/filters.py @@ -0,0 +1,61 @@ +# Patchwork - automated patch tracking system +# Copyright (C) 2016 Stephen Finucane <stephen@that.guru> +# +# This file is part of the Patchwork package. +# +# Patchwork is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# Patchwork is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Patchwork; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + +from django_filters import FilterSet +from django_filters import IsoDateTimeFilter + +from patchwork.models import Check +from patchwork.models import CoverLetter +from patchwork.models import Patch +from patchwork.models import Series + + +class TimestampMixin(object): + + # TODO(stephenfin): These should filter on a 'updated_at' field instead + before = IsoDateTimeFilter(name='date', lookup_expr='lt') + since = IsoDateTimeFilter(name='date', lookup_expr='gte') + + +class SeriesFilter(TimestampMixin, FilterSet): + + class Meta: + model = Series + fields = ['submitter'] + + +class CoverLetterFilter(TimestampMixin, FilterSet): + + class Meta: + model = CoverLetter + fields = ['series', 'submitter'] + + +class PatchFilter(FilterSet): + + class Meta: + model = Patch + fields = ['series', 'submitter', 'delegate', 'state', 'archived'] + + +class CheckFilter(TimestampMixin, FilterSet): + + class Meta: + model = Check + fields = ['user', 'state', 'context'] diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py index 8d308e8..c1c5030 100644 --- a/patchwork/api/patch.py +++ b/patchwork/api/patch.py @@ -27,6 +27,7 @@ from rest_framework.serializers import ChoiceField from rest_framework.serializers import HyperlinkedModelSerializer from rest_framework.serializers import SerializerMethodField +from patchwork.api.filters import PatchFilter from patchwork.api import PatchworkPermission from patchwork.api import STATE_CHOICES from patchwork.models import Patch @@ -118,6 +119,7 @@ class PatchList(ListAPIView): 'content', 'diff', 'headers') permission_classes = (PatchworkPermission,) serializer_class = PatchListSerializer + filter_class = PatchFilter class PatchDetail(RetrieveUpdateAPIView): diff --git a/patchwork/api/series.py b/patchwork/api/series.py index fead4ca..a7f1344 100644 --- a/patchwork/api/series.py +++ b/patchwork/api/series.py @@ -21,6 +21,7 @@ from rest_framework.generics import ListAPIView from rest_framework.generics import RetrieveAPIView from rest_framework.serializers import HyperlinkedModelSerializer +from patchwork.api.filters import SeriesFilter from patchwork.api import PatchworkPermission from patchwork.models import Series @@ -51,7 +52,7 @@ class SeriesMixin(object): class SeriesList(SeriesMixin, ListAPIView): """List series.""" - pass + filter_class = SeriesFilter class SeriesDetail(SeriesMixin, RetrieveAPIView): diff --git a/patchwork/settings/base.py b/patchwork/settings/base.py index 35ac96a..dd2d3d0 100644 --- a/patchwork/settings/base.py +++ b/patchwork/settings/base.py @@ -133,7 +133,8 @@ try: import rest_framework # NOQA INSTALLED_APPS += [ - 'rest_framework' + 'rest_framework', + 'django_filters', ] except ImportError: pass @@ -144,6 +145,9 @@ REST_FRAMEWORK = { 'rest_framework.versioning.NamespaceVersioning' ), 'DEFAULT_PAGINATION_CLASS': 'patchwork.api.LinkHeaderPagination', + 'DEFAULT_FILTER_BACKENDS': ( + 'django_filters.rest_framework.DjangoFilterBackend', + ) } # diff --git a/requirements-test.txt b/requirements-test.txt index 7cb5ae9..c664976 100644 --- a/requirements-test.txt +++ b/requirements-test.txt @@ -3,4 +3,4 @@ django-debug-toolbar==1.5 python-dateutil>2.0,<3.0 selenium>2.0,<3.0 djangorestframework>=3.4,<3.5 -drf-nested-routers>=0.11.1,<0.12 +django-filter>=0.15,<0.16
This mostly works out of the box, thanks to Django REST Framework. Mostly unique fields, like name or email, are excluded as these will be handled separately. Signed-off-by: Stephen Finucane <stephen@that.guru> --- patchwork/api/check.py | 2 ++ patchwork/api/cover.py | 2 ++ patchwork/api/filters.py | 61 ++++++++++++++++++++++++++++++++++++++++++++++ patchwork/api/patch.py | 2 ++ patchwork/api/series.py | 3 ++- patchwork/settings/base.py | 6 ++++- requirements-test.txt | 2 +- 7 files changed, 75 insertions(+), 3 deletions(-) create mode 100644 patchwork/api/filters.py