django-rest-framework (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation4 Commits3 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

@rpkilby OK, too early... Tests pass but if __getattribute__ raises the AttributeError, why is __getattr__ not then (re-)called?

I might have this completely wrong, as I don't know much about the cpython internals, but... my understanding is that the underlying c code calls __getattribute__ and if an AttributeError is raised, it then calls __getattr__ (if defined). The default implementation of __getattribute__ isn't directly calling __getattr__, which is why we don't see an infinite loop.

In essence:

Underlying C code calls Request.__getattribute__ which fails and raises an AttributeError.
Underlying C code calls Request.__getattr__, which checks the WSGIRequest.
WSGIRequest check fails, so Request.__getattr__ calls Request.__getattribute__ to raise the exception.

I just did a simple timeit test, and the PR does seem to be faster (which makes sense, since there are less caught exceptions. I used the following test:

class Perf(TestCase): def test_perf(self): from timeit import repeat

    wsgi_request = factory.get('/')
    request = Request(wsgi_request)

    # non-existent attribute
    def foo():
        try:
            request.foo
        except AttributeError:
            pass

    # attribute exists on WSGIRequest
    def body():
        request.body

    # attribute exists on DRF Request
    def _request():
        request._request

    print(min(repeat(foo, number=100000)))
    print(min(repeat(body, number=100000)))
    print(min(repeat(_request, number=100000)))

    raise Exception('done')

Timings	foo	body	_request
before	0.57 s	0.36 s	0.07 s
after	0.25 s	0.13 s	0.01 s

OK. It does read that way. Good linking skills.

rpkilby deleted the request-attribute-access branch

November 22, 2017 17:20

pchiquet pushed a commit to pchiquet/django-rest-framework that referenced this pull request

Nov 17, 2020

Add tests for proxying WSGIRequest attributes in Request.
Add request attribute exception test
Reimplement request attribute access