Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The framework is itself using its own deprecated (fatal error) properties #3239

Closed
hakanw opened this issue Aug 7, 2015 · 6 comments
Closed
Milestone

Comments

@hakanw
Copy link

hakanw commented Aug 7, 2015

This method now dies, because .FILES is no longer working at all.

https://github.com/tomchristie/django-rest-framework/blob/master/rest_framework/request.py#L338

@tomchristie
Copy link
Member

What the? Darn. How does that not get caught anywhere?

@tomchristie
Copy link
Member

No, no, no - this is fine - we're accessing .FILES on the Django's underlying HTTPRequest instance.

How are you triggering the error - presumably must be doing something futzy like calling directly into a view from another?

@illing2005
Copy link

I think the deprecation of request.FILES is problematic.
For instance https://github.com/Tivix/django-rest-auth isn't working anymore, because it relies on all_auth which uses django.view.generics. And there request.FILES is used.
For instance in site-packages/django/views/generic/edit.py

    def get_form_kwargs(self):
    """
    Returns the keyword arguments for instantiating the form.
    """
    kwargs = {
        'initial': self.get_initial(),
        'prefix': self.get_prefix(),
    }

    if self.request.method in ('POST', 'PUT'):
        kwargs.update({
            'data': self.request.POST,
            'files': self.request.FILES,
        })
    return kwargs

`

@hakanw
Copy link
Author

hakanw commented Aug 7, 2015

No, I get an exception during authentication due to this:

  File "/Users/hakan/src/active_projects/P5/app/views.py", line 216, in auth
    resp = obtain_auth_token(request)
  File "/Users/hakan/.virtualenvs/opinion3/lib/python2.7/site-packages/django/views/decorators/csrf.py", line 58, in wrapped_view
    return view_func(*args, **kwargs)
  File "/Users/hakan/.virtualenvs/opinion3/lib/python2.7/site-packages/django/views/generic/base.py", line 71, in view
    return self.dispatch(request, *args, **kwargs)
  File "/Users/hakan/.virtualenvs/opinion3/lib/python2.7/site-packages/rest_framework/views.py", line 464, in dispatch
    response = self.handle_exception(exc)
  File "/Users/hakan/.virtualenvs/opinion3/lib/python2.7/site-packages/rest_framework/views.py", line 455, in dispatch
    if request.method.lower() in self.http_method_names:
  File "/Users/hakan/.virtualenvs/opinion3/lib/python2.7/site-packages/rest_framework/request.py", line 444, in __getattribute__
    return super(Request, self).__getattribute__(attr)
  File "/Users/hakan/.virtualenvs/opinion3/lib/python2.7/site-packages/rest_framework/request.py", line 174, in method
    self._load_method_and_content_type()
  File "/Users/hakan/.virtualenvs/opinion3/lib/python2.7/site-packages/rest_framework/request.py", line 287, in _load_method_and_content_type
    self._perform_form_overloading()
  File "/Users/hakan/.virtualenvs/opinion3/lib/python2.7/site-packages/rest_framework/request.py", line 338, in _perform_form_overloading
    self._files = self._request.FILES
  File "/Users/hakan/.virtualenvs/opinion3/lib/python2.7/site-packages/rest_framework/request.py", line 444, in __getattribute__
    return super(Request, self).__getattribute__(attr)
  File "/Users/hakan/.virtualenvs/opinion3/lib/python2.7/site-packages/rest_framework/request.py", line 462, in FILES
    '`request.FILES` has been deprecated in favor of `request.files` '

@tomchristie
Copy link
Member

I'm not convinced that there's not some less obvious issue with the calling code in the example above, but either way around, let's leave .FILES in place for compat with how standard Django requests are used (unlike .DATA and .QUERY_PARAMS which are property names that only belong the REST framework).

We have had a pending deprecation warning on this since 3.0 and a deprecation warning on 3.1, so it's a little tedious that it's not been raised as an issue until we remove it completely.

@tomchristie tomchristie added this to the 3.2.1 Release milestone Aug 7, 2015
@hakanw
Copy link
Author

hakanw commented Aug 7, 2015

Thanks for the swift commit. Yes, I have no idea why that path is being called. I'm just using django-rest-framework, but I do have django-all-auth also. I wonder if that is somehow involved here? Very strange!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants