[Django] #28358: LazyObject defines attribute that don't exist on wrapped object
Django
-------------------------------------------+------------------------
Reporter: Andrey Fedoseev | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: 1.11
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------------+------------------------
`LazyObject` defines magic methods (`__getitem__`, `__iter__`) which may
be missing from the wrapped object. This leads to the following errors:
{{{#!python
some_variable = request.user
if hasattr(some_variable, "__getitem__"):
foo = some_variable["foo"] # raises TypeError: 'User' object has no
attribute '__getitem__'
if hasattr(some_variable, "__iter__"):
for item in some_variable: # raises TypeError: 'User' object is not
iterable
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28358>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Andrey Fedoseev | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: 1.11
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/28358#comment:1>
Django
Reporter: Andrey Fedoseev | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: 1.11
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Sergey Fedoseev):
* cc: Sergey Fedoseev (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/28358#comment:2>
Django
Reporter: Andrey Fedoseev | Owner: Sergey
| Fedoseev
Type: Bug | Status: assigned
Component: Utilities | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Sergey Fedoseev):
* status: new => assigned
* owner: nobody => Sergey Fedoseev
--
Ticket URL: <https://code.djangoproject.com/ticket/28358#comment:3>
Django
Reporter: Andrey Fedoseev | Owner: Sergey
| Fedoseev
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Sergey Fedoseev):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/8806 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/28358#comment:4>
Django
Reporter: Andrey Fedoseev | Owner: Sergey
| Fedoseev
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/28358#comment:5>
Django
Reporter: Andrey Fedoseev | Owner: (none)
Type: Bug | Status: new
Component: Utilities | Version: 1.11
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* owner: Sergey Fedoseev => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/28358#comment:6>
Django
Reporter: Andrey Fedoseev | Owner: (none)
Component: Utilities | Version: 1.11
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Sergey Fedoseev):
* cc: Sergey Fedoseev (removed)
--
Ticket URL: <https://code.djangoproject.com/ticket/28358#comment:7>
Django
Reporter: Andrey Fedoseev | Owner: Theofilos
| Alexiou
Type: Bug | Status: assigned
Component: Utilities | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Theofilos Alexiou):
* owner: (none) => Theofilos Alexiou
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/28358#comment:8>
Django
Reporter: Andrey Fedoseev | Owner: Theofilos
| Alexiou
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Theofilos Alexiou):
* needs_better_patch: 1 => 0
Comment:
[https://github.com/django/django/pull/15423 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/28358#comment:9>
Django
Reporter: Andrey Fedoseev | Owner: Theofilos
| Alexiou
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/28358#comment:10>
Django
Reporter: Andrey Fedoseev | Owner: Theofilos
| Alexiou
Component: Utilities | Version: 1.11
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"97d7990abde3fe4b525ae83958fd0b52d6a1d13f" 97d7990a]:
{{{
#!CommitTicketReference repository=""
revision="97d7990abde3fe4b525ae83958fd0b52d6a1d13f"
Fixed #28358 -- Prevented LazyObject from mimicking nonexistent
attributes.
Thanks Sergey Fedoseev for the initial patch.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28358#comment:11>
Django
Reporter: Andrey Fedoseev | Owner: Theofilos
| Alexiou
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"b2ed0d78f2dff9986ef15b9098c1b6d9ce720a99" b2ed0d78]:
{{{
#!CommitTicketReference repository=""
revision="b2ed0d78f2dff9986ef15b9098c1b6d9ce720a99"
Refs #28358 -- Fixed infinite recursion in LazyObject.__getattribute__().
Regression in 97d7990abde3fe4b525ae83958fd0b52d6a1d13f.
Co-authored-by: Mariusz Felisiak <felisiak...@gmail.com>
Co-authored-by: Theo Alexiou <theofilo...@gmail.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28358#comment:12>
Django
Reporter: Andrey Fedoseev | Owner: Theofilos
| Alexiou
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Dylan Young):
I'm not 100%, but it looks like these fixes might have broken Django on
PyPy.
{{{
django-csp/.tox/pypy3.9-main/lib/pypy3.9/site-
packages/django/contrib/admin/decorators.py", line 107, in
_model_admin_wrapper
raise ValueError("site must subclass AdminSite")
ValueError: site must subclass AdminSite
}}}
mro looks like this (not sure what's expected)
{{{
[<class 'django.contrib.admin.sites.DefaultAdminSite'>, <class
'django.utils.functional.LazyObject'>, <class 'object'>]
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28358#comment:13>
Django
Reporter: Andrey Fedoseev | Owner: Theofilos
| Alexiou
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak):
Dylan, Can you confirm that it works for you with Django 4.0? Which commit
introduces a regression? Is it not an issue in `pypy` itself?
--
Ticket URL: <https://code.djangoproject.com/ticket/28358#comment:14>
Django
Reporter: Andrey Fedoseev | Owner: Theofilos
| Alexiou
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak):
Replying to [comment:13 Dylan Young]:
> Works fine in 4.0.
>
> Created a PyPy issue here
https://foss.heptapod.net/pypy/pypy/-/issues/3751
Basic test project works for me on PyPy and Django 4.0+.
--
Ticket URL: <https://code.djangoproject.com/ticket/28358#comment:15>
Django
Reporter: Andrey Fedoseev | Owner: Theofilos
| Alexiou
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Theofilos Alexiou):
Replying to [comment:13 Dylan Young]:
> I'm not 100%, but it looks like these fixes might have broken Django on
PyPy.
>
>
>
> {{{
> django-csp/.tox/pypy3.9-main/lib/pypy3.9/site-
packages/django/contrib/admin/decorators.py", line 107, in
_model_admin_wrapper
> raise ValueError("site must subclass AdminSite")
> ValueError: site must subclass AdminSite
> }}}
Not sure if this is of any help or not. The error is raised on line 107
but looking at the 4.1 brach I see that this should be in line
[https://github.com/django/django/blob/9fc56af0fbd77253bd8e90b8478e32b0d30fc6b9/django/contrib/admin/decorators.py#L102
102] Is this perhaps a modified brach of Django and some other change
causes it?
--
Ticket URL: <https://code.djangoproject.com/ticket/28358#comment:16>
Django
Reporter: Andrey Fedoseev | Owner: Theofilos
| Alexiou
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Dylan Young):
It's main, not 4.1. I'll try 4.1 and see if it shows up there too.
Replying to [comment:16 Theofilos Alexiou]:
> Replying to [comment:13 Dylan Young]:
> > I'm not 100%, but it looks like these fixes might have broken Django
on PyPy.
> >
> >
> >
> > {{{
> > django-csp/.tox/pypy3.9-main/lib/pypy3.9/site-
packages/django/contrib/admin/decorators.py", line 107, in
_model_admin_wrapper
> > raise ValueError("site must subclass AdminSite")
> > ValueError: site must subclass AdminSite
> > }}}
>
> Not sure if this is of any help or not. The error is raised on line 107
but looking at the 4.1 brach I see that this should be in line
[https://github.com/django/django/blob/9fc56af0fbd77253bd8e90b8478e32b0d30fc6b9/django/contrib/admin/decorators.py#L102
102] Is this perhaps a modified brach of Django and some other change
causes it?
--
Ticket URL: <https://code.djangoproject.com/ticket/28358#comment:17>
Django
Reporter: Andrey Fedoseev | Owner: Theofilos
| Alexiou
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Dylan Young):
Replicated on 4.1a1
{{{
File "django-csp/.tox/pypy3.9-4.1.x/lib/pypy3.9/site-
packages/django/contrib/auth/admin.py", line 29, in <module>
class GroupAdmin(admin.ModelAdmin):
File "django-csp/.tox/pypy3.9-4.1.x/lib/pypy3.9/site-
packages/django/contrib/admin/decorators.py", line 102, in
_model_admin_wrapper
raise ValueError("site must subclass AdminSite")
ValueError: site must subclass AdminSite
}}}
I was able to narrow this down to some kind of interaction with coverage
or pytest-cov (see https://foss.heptapod.net/pypy/pypy/-/issues/3751).
There's a larger traceback on that issue as well.
--
Ticket URL: <https://code.djangoproject.com/ticket/28358#comment:18>
Django
Reporter: Andrey Fedoseev | Owner: Theofilos
| Alexiou
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Dylan Young):
Introduced by 97d7990abde3fe4b525ae83958fd0b52d6a1d13f
Confirmed 1d071ec1aa8fa414bb96b41f7be8a1bd01079815 does not exhibit the
error.
--
Ticket URL: <https://code.djangoproject.com/ticket/28358#comment:19>
Django
Reporter: Andrey Fedoseev | Owner: Theofilos
| Alexiou
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Dylan Young):
Yes, I would guess that it's an issue with PyPy or perhaps `coverage` or
`pytest-cov` using non-spec CPython implementation details, but that's
just a guess.
Replying to [comment:14 Mariusz Felisiak]:
> Dylan, Can you confirm that it works for you with Django 4.0? Which
commit introduces a regression? Is it not an issue in `pypy` itself?
--
Ticket URL: <https://code.djangoproject.com/ticket/28358#comment:20>
Django
Reporter: Andrey Fedoseev | Owner: Theofilos
| Alexiou
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by cfbolz):
hi everyone! PyPy dev here. So after some investigation (you can find the
gory details on this CPython issue:
https://github.com/python/cpython/issues/98148 ) it turns out that this
bug also exists in CPython, if you run coverage in pure python mode. The
bug occurs only if you: 1) use `super` 2) define `__class__` in the class
body yourself 3) use a pure python trace hook, or call locals() in the
class body.
At this point there has been no reaction from CPython yet, but the way I
see things from the PyPy side it'll be on the tricky to fix this so it
might take a while (I'm waiting for CPy feedback because the two
implementations use exactly the same approaches here, to the point of
ending up with exactly the same bug).
I was wondering whether Django might be open to a PR that works around the
bug for the time being? A pragmatic approach would be to stop using super
in `LazyObject.__getattribute__`, which was introduced in
97d7990abde3fe4b525ae83958fd0b52d6a1d13f, and just call
`object.__getattribute__` instead. If that sounds like a reasonable
solution, I am happy to work on the PR. (there would be some trickiness to
how to unit test this, but I can probably think of something). Please let
me know!
--
Ticket URL: <https://code.djangoproject.com/ticket/28358#comment:21>
Django
Reporter: Andrey Fedoseev | Owner: Theofilos
| Alexiou
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Collin Anderson):
* cc: Collin Anderson (added)
Comment:
the `object.__getattribute__(self, name)` workaround mentioned above fixed
the issue for me running pure-python coverage.
I made a basic PR but don't plan on pushing it through to the end. I'll
opt for actually installing coverage rather than running it straight from
a git checkout but at least wanted to report that the workaround does
work.
https://github.com/django/django/pull/16541
For testing, somehow running a pure-python version of coverage as part of
continuous integration would show the issue, though ideally you want to
test both with and without c-speedups.
--
Ticket URL: <https://code.djangoproject.com/ticket/28358#comment:22>