[7/7] REST: Use DRF-specific filterset

Message ID 20180411161338.420-8-stephen@that.guru
State Accepted
Headers show
Series
  • Add support for multiple filters
Related show

Commit Message

Stephen Finucane April 11, 2018, 4:13 p.m.
Better error handling, yo.

Signed-off-by: Stephen Finucane <stephen@that.guru>
---
 patchwork/api/filters.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Daniel Axtens May 8, 2018, 3:56 p.m. | #1
Stephen Finucane <stephen@that.guru> writes:

> Better error handling, yo.

I don't know the difference between the django_filters.FilterSet and the
django_filters.rest_framework.FilterSet

If it's error handling, does this need to be earlier in the patch
series? Your patch 3/7 for example does some error handling I don't
recognise, so I just wanted to check for the sake of bisectability and
good software practice...

Regards,
Daniel
>
> Signed-off-by: Stephen Finucane <stephen@that.guru>
> ---
>  patchwork/api/filters.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/patchwork/api/filters.py b/patchwork/api/filters.py
> index 4d8d504d..f6fff792 100644
> --- a/patchwork/api/filters.py
> +++ b/patchwork/api/filters.py
> @@ -20,7 +20,7 @@
>  from django.contrib.auth.models import User
>  from django.core.exceptions import ValidationError
>  from django.db.models import Q
> -from django_filters import FilterSet
> +from django_filters.rest_framework import FilterSet
>  from django_filters import IsoDateTimeFilter
>  from django_filters import ModelMultipleChoiceFilter
>  from django.forms import ModelMultipleChoiceField as BaseMultipleChoiceField
> -- 
> 2.14.3
>
> _______________________________________________
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
Stephen Finucane May 8, 2018, 4:21 p.m. | #2
On Wed, 2018-05-09 at 01:56 +1000, Daniel Axtens wrote:
> Stephen Finucane <stephen@that.guru> writes:
> 
> > Better error handling, yo.
> 
> I don't know the difference between the django_filters.FilterSet and the
> django_filters.rest_framework.FilterSet
> 
> If it's error handling, does this need to be earlier in the patch
> series? Your patch 3/7 for example does some error handling I don't
> recognise, so I just wanted to check for the sake of bisectability and
> good software practice...

To be honest, this is more of a "because the docs told me to do this"
thing, hence the lack of context. This variant of 'FilterSet' will
convert a 'django.forms.ValidationError', which wouldn't be handled by
DRF, to a 'rest_framework.exceptions.ValidationError', which would be.
It also enables pretty filtering if 'django-crispy-forms' is installed
but that's not an issue for us. I don't think there's any reason to
move it any earlier in the series though because this is an existing
issue that none of the prior patches are making any better/worse.

Stephen

> Regards,
> Daniel
> > 
> > Signed-off-by: Stephen Finucane <stephen@that.guru>
> > ---
> >  patchwork/api/filters.py | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/patchwork/api/filters.py b/patchwork/api/filters.py
> > index 4d8d504d..f6fff792 100644
> > --- a/patchwork/api/filters.py
> > +++ b/patchwork/api/filters.py
> > @@ -20,7 +20,7 @@
> >  from django.contrib.auth.models import User
> >  from django.core.exceptions import ValidationError
> >  from django.db.models import Q
> > -from django_filters import FilterSet
> > +from django_filters.rest_framework import FilterSet
> >  from django_filters import IsoDateTimeFilter
> >  from django_filters import ModelMultipleChoiceFilter
> >  from django.forms import ModelMultipleChoiceField as BaseMultipleChoiceField
> > -- 
> > 2.14.3
> > 
> > _______________________________________________
> > Patchwork mailing list
> > Patchwork@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/patchwork

Patch

diff --git a/patchwork/api/filters.py b/patchwork/api/filters.py
index 4d8d504d..f6fff792 100644
--- a/patchwork/api/filters.py
+++ b/patchwork/api/filters.py
@@ -20,7 +20,7 @@ 
 from django.contrib.auth.models import User
 from django.core.exceptions import ValidationError
 from django.db.models import Q
-from django_filters import FilterSet
+from django_filters.rest_framework import FilterSet
 from django_filters import IsoDateTimeFilter
 from django_filters import ModelMultipleChoiceFilter
 from django.forms import ModelMultipleChoiceField as BaseMultipleChoiceField