#28358 closed Bug (fixed)
LazyObject defines attribute that don't exist on wrapped object
Reported by: | Andrey Fedoseev | Owned by: | Theofilos Alexiou |
---|---|---|---|
Component: | Utilities | Version: | 1.11 |
Severity: | Normal | Keywords: | |
Cc: | Collin Anderson | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
LazyObject
defines magic methods (__getitem__
, __iter__
) which may be missing from the wrapped object. This leads to the following errors:
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
Change History (23)
comment:1 by , 7 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 7 years ago
Cc: | added |
---|
comment:3 by , 7 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:4 by , 7 years ago
Has patch: | set |
---|
comment:5 by , 7 years ago
Patch needs improvement: | set |
---|
comment:6 by , 3 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:7 by , 3 years ago
Cc: | removed |
---|
comment:8 by , 3 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:10 by , 3 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
follow-ups: 15 16 comment:13 by , 2 years ago
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'>]
Works fine in 4.0.
Created a PyPy issue here https://foss.heptapod.net/pypy/pypy/-/issues/3751
follow-up: 20 comment:14 by , 2 years ago
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?
comment:15 by , 2 years ago
Replying to 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+.
follow-up: 17 comment:16 by , 2 years ago
Replying to 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 102 Is this perhaps a modified brach of Django and some other change causes it?
comment:17 by , 2 years ago
It's main, not 4.1. I'll try 4.1 and see if it shows up there too.
Replying to Theofilos Alexiou:
Replying to 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 AdminSiteNot 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 102 Is this perhaps a modified brach of Django and some other change causes it?
comment:18 by , 2 years ago
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.
4.0 is fine. Not sure which commit introduces it at this point. These ones just seemed like the likely culprit.
LOL, it was probably my debugging code throwing off the line numbers:
# admin_site = admin_site._wrapped # print(admin_site, type(admin_site), type(admin_site).mro()) # for klass in type(admin_site).mro(): # print(klass) # print(dir(klass))
comment:19 by , 2 years ago
Introduced by 97d7990abde3fe4b525ae83958fd0b52d6a1d13f
Confirmed 1d071ec1aa8fa414bb96b41f7be8a1bd01079815 does not exhibit the error.
comment:20 by , 2 years ago
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 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?
comment:21 by , 23 months ago
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!
comment:22 by , 21 months ago
Cc: | added |
---|
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.
Edit: Also here's a direct link to the Python issue: https://github.com/python/cpython/issues/98148
comment:23 by , 6 months ago
The PyPy issue for the ValueError: site must subclass AdminSite
exception has moved to https://github.com/pypy/pypy/issues/3750.
I've just opened #35418 to track adding the workaround.
PR