Deprecate DjangoFilterBackend

[rpkilby]

Attempts to resolve #3371. The deprecation assumes/asserts the presence of the migrated django-filter backend, which requires the recent 0.15 release. Because of the dependency version bump, it might be appropriate to save this for DRF 3.5?

[tomchristie]

Because of the dependency version bump, it might be appropriate to save this for DRF 3.5?

Haven't yet reviewed, but yes agreed on this point.

[carltongibson]

Looks great!

[carltongibson]

Looks like a test on get_fields would fix the patch coverage failure.

[rpkilby]

Given my comment here, I'm not really sure how to write a meaningful test.

[carltongibson]

@tomchristie a small amount of guidance if you please... :-)

[tomchristie]

This allows introspection of filter fields for schema generation. It's possible that we ought to change the signature prior to 3.5, so that it returns more information that simply the names. Currently undecided on that.

[rpkilby]

So - just leave it as is for now?

[tomchristie]

Yup.

[rpkilby]

Hi @tomchristie. The 3.5 release seems imminent - did you want to merge this in for 3.5 or hold of until later? Aside from fixing conflicts, anything else I should clean up?

[tomchristie]

Milestoning to ensure we review this as part of 3.5. May or may not make it in.

Initial thoughts are that the change footprint looks much larger than expected.

Also could we get some details on what users need to do in order to replace this with the django-filter version - what does that involve exactly?

[tomchristie]

Also for consideration - if it's available as a third party package, then we might prefer to simple remove it as long as the drop-in replacement is easy to make.

[carltongibson]

Using the new FilterSet simply requires changing the import path

https://django-filter.readthedocs.io/en/latest/rest_framework.html

It should Just Work™ — happy to take Issues if it doesn't.

[carltongibson]

# settings.py
REST_FRAMEWORK = {
   'DEFAULT_FILTER_BACKENDS': (
       'django_filters.rest_framework.DjangoFilterBackend',
       ...
   ),
}

Can DRF (conditionally) make this the default setting?

[kevin-brown]

Initial thoughts are that the change footprint looks much larger than expected.

This mostly appears to be because this is removing the old DjangoFilterBackend and instead importing the new one from django-filter instead. Apparently we had a lot of code that was required only to support the DjangoFilterBackend that could be removed.

[tomchristie]

If we do decide to do this, I'm not sure why we'd decide to keep the stub class.
Better to fully remove it, and defer everything to the third party package, right?

Does the get_fields change need to be applied to the latest version of django-filter still, or is that already up on PyPI?

[carltongibson]

1. The idea was to deprecate — and raise the warning — just to allow people time to adjust — but some adjustment is expected and it's only minor. @xordoquy: You were the voice of reason here, what say you?
2. No. I need to look at/update/merge Port get_schema_fields from DRF carltongibson/django-filter#492 and then it should be good to go.

[xordoquy]

Hi there.

My take on this would be relatively simple:

• Keep the DjangoFilterBackend as it used to be.
• Add a deprecation warning whenever it is instantiated such as:

class BaseFilterBackend(object):
    def __init__(self):
         assert django_filters, 'Using DjangoFilterBackend, but django-filter is not installed'
         warnings.warn("'rest_framework.filters.DjangoFilterBackend' is depracated. Use 'django_filters.rest_framework.DjangoFilterBackend' with django-filter>0.15", PendingDeprecationWarning)
)

• add this to the release notes.

Once 3.5 is out, we'll raise this to DeprecationWarning and finally remove it totally in 3.7 (i.e. after 3.6).

What I'm not sure about all this is about how compatible the current code is with Django-filters.

[carltongibson]

What I'm not sure about all this is about how compatible the current code is with Django-filters.

Almost entirely... as per the comment, it should be "change the import and you're good to go".

It was a straight migration of the the DRF code and tests.

Since added there's a slight change to the filter defaults which make boolean and date handling more API friendly — this was the No1 most requested thing in the world — but that's it.

[carltongibson]

This was the commit which pulled over the DRF code

[rpkilby]

@xordoquy - my thoughts on removing the code is to prevent having to maintain the backend in both packages. For example, see #4574 & carltongibson/django-filter#521.

Would this deprecation timeline make sense?

• For 3.5, add a pending deprecation warning per @xordoquy's comment, leaving the backend for now.
• For 3.6, bump to deprecation warning and merge the remainder of this PR, wrapping the backend from django-filter.
• For 3.7, remove the backend from DRF altogether.

[xordoquy]

In my opinion, this side of the code shouldn't be maintain. It's deprecated and would remain just for deprecation policy. The answer to bugfix being use the django filter's solution