LazyObject defines attribute that don't exist on wrapped object

[Andrey Fedoseev]

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

[Sergey Fedoseev]

​PR

[Theofilos Alexiou]

​PR

[Mariusz Felisiak <felisiak.mariusz@…>]

In 97d7990a:

Fixed #28358 -- Prevented LazyObject from mimicking nonexistent attributes.

Thanks Sergey Fedoseev for the initial patch.

[Mariusz Felisiak <felisiak.mariusz@…>]

In b2ed0d78:

Refs #28358 -- Fixed infinite recursion in LazyObject.getattribute().

Regression in 97d7990abde3fe4b525ae83958fd0b52d6a1d13f.

Co-authored-by: Mariusz Felisiak <felisiak.mariusz@…>
Co-authored-by: Theo Alexiou <theofilosalexiou@…>

[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'>]

Works fine in 4.0.

Created a PyPy issue here ​https://foss.heptapod.net/pypy/pypy/-/issues/3751

[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?

[Mariusz Felisiak]

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+.

[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 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?

[Dylan Young]

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 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?

[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.

4.0 is fine. Not sure which commit introduces it at this point. These ones just seemed like the likely culprit.

[Dylan Young]

Introduced by 97d7990abde3fe4b525ae83958fd0b52d6a1d13f

Confirmed 1d071ec1aa8fa414bb96b41f7be8a1bd01079815 does not exhibit the error.

[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 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?

[Carl Friedrich Bolz-Tereick]

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!

[Collin Anderson]

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

[Lily Foote]

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.