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

Type[SomeConcreteModel] has no attribute "objects" -- but it does at runtime #1744

Open
asottile-sentry opened this issue Sep 28, 2023 · 7 comments · May be fixed by #2280
Open

Type[SomeConcreteModel] has no attribute "objects" -- but it does at runtime #1744

asottile-sentry opened this issue Sep 28, 2023 · 7 comments · May be fixed by #2280
Labels
bug Something isn't working

Comments

@asottile-sentry
Copy link

Bug report

unfortunately having a really hard time tracking this down -- I've seen #1684 already however this is a concrete model and not an abstract one

What's wrong

I haven't been able to narrow this to a minimal case so I suspect it's something wrong with our setup -- here's at least a maximal reproduction:

set -euxo pipefail

rm -rf sentry

git clone -qq --depth=1 https://github.com/getsentry/sentry -b 23.9.1 >& /dev/null
cd sentry
python3.8 -m venv venv >& /dev/null
# cheaty editable install so you don't need node
echo "$PWD/src" > venv/lib/python3.8/site-packages/easy-install.pth
venv/bin/pip install -qq --no-deps -r requirements-dev-frozen.txt
venv/bin/pip install -qq --upgrade mypy django-stubs django-stubs-ext
venv/bin/mypy tests/sentry/db/models/fields/test_jsonfield.py

output:

$ bash t.sh 
+ rm -rf sentry
+ git clone -qq --depth=1 https://github.com/getsentry/sentry -b 23.9.1
+ cd sentry
+ python3.8 -m venv venv
+ echo /tmp/y/sentry/src
+ venv/bin/pip install -qq --no-deps -r requirements-dev-frozen.txt
+ venv/bin/pip install -qq --upgrade mypy django-stubs django-stubs-ext
+ venv/bin/mypy tests/sentry/db/models/fields/test_jsonfield.py
tests/sentry/db/models/fields/test_jsonfield.py:58: error: "Type[JSONFieldTestModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:64: error: "Type[JSONFieldTestModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:68: error: "Type[JSONFieldTestModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:69: error: "Type[JSONFieldTestModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:119: error: "Type[JSONFieldWithDefaultTestModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:120: error: "Type[JSONFieldWithDefaultTestModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:124: error: "Type[JSONFieldTestModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:125: error: "Type[JSONFieldTestModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:126: error: "Type[JSONFieldTestModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:127: error: "Type[JSONFieldTestModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:128: error: "Type[JSONFieldTestModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:129: error: "Type[JSONFieldTestModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:131: error: "Type[JSONFieldTestModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:133: error: "Type[JSONFieldTestModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:135: error: "Type[JSONFieldTestModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:139: error: "Type[JSONFieldTestModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:142: error: "Type[JSONFieldTestModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:146: error: "Type[JSONFieldTestModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:147: error: "Type[JSONFieldTestModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:148: error: "Type[JSONFieldTestModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:150: error: "Type[JSONFieldTestModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:151: error: "Type[JSONFieldTestModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:154: error: "Type[BlankJSONFieldTestModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:155: error: "Type[BlankJSONFieldTestModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:159: error: "Type[BlankJSONFieldTestModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:164: error: "Type[CallableDefaultModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:165: error: "Type[CallableDefaultModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:169: error: "Type[CallableDefaultModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:170: error: "Type[CallableDefaultModel]" has no attribute "objects"  [attr-defined]
tests/sentry/db/models/fields/test_jsonfield.py:194: error: "Type[BlankJSONFieldTestModel]" has no attribute "objects"  [attr-defined]
Found 30 errors in 1 file (checked 1 source file)

will try and narrow it down to a minimal case but I've failed to do that so far

How is that should be

I expect no errors in that file

System information

  • OS:
  • python version: 3.8.16
  • django version: 3.2.20
  • mypy version: 1.5.1
  • django-stubs version: 4.2.4
  • django-stubs-ext version: 4.2.2
@asottile-sentry asottile-sentry added the bug Something isn't working label Sep 28, 2023
@asottile-sentry
Copy link
Author

asottile-sentry commented Sep 28, 2023

partially reverting #1672 fixes this so I suspect the removal of objects there isn't correct CC @flaeppe

--- a/mypy_django_plugin/transformers/models.py
+++ b/mypy_django_plugin/transformers/models.py
@@ -833,9 +833,9 @@ class MetaclassAdjustments(ModelClassInitializer):
         ):
             del ctx.cls.info.names["MultipleObjectsReturned"]
 
-        objects = ctx.cls.info.names.get("objects")
-        if objects is not None and isinstance(objects.node, Var) and not objects.plugin_generated:
-            del ctx.cls.info.names["objects"]
+        # objects = ctx.cls.info.names.get("objects")
+        # if objects is not None and isinstance(objects.node, Var) and not objects.plugin_generated:
+        #     del ctx.cls.info.names["objects"]
 
         return
 

@flaeppe
Copy link
Member

flaeppe commented Sep 28, 2023

I was looking at this a little earlier and I'm suspecting there's a configuration mismatch involved. I wasn't able to start up sentry though, but I admin I didn't try too hard either.

Anyways, I think all models within that file are test models but the django-stubs plugin relies heavily (unfortunately) on the runtime. So unless I'm reading stuff wrong, I'm suspecting that the runtime that django-stubs loads doesn't include the models specified in tests/sentry/db/models/fields/test_jsonfield.py? (Essentially since their app isn't in INSTALLED_APPS)

If they don't, the plugin will now delete objects but never add it back.. So perhaps the plugin should be more careful before deleting the objects attribute from what it sees as non-registered models..

I haven't verified this, but I think if one would set up an editable install of django-stubs and place a strategic print or so in here:

@cached_property
def model_modules(self) -> Dict[str, Dict[str, Type[Model]]]:
"""All modules that contain Django models."""
modules: Dict[str, Dict[str, Type[Model]]] = defaultdict(dict)
for concrete_model_cls in self.apps_registry.get_models():
modules[concrete_model_cls.__module__][concrete_model_cls.__name__] = concrete_model_cls
# collect abstract=True models
for model_cls in concrete_model_cls.mro()[1:]:
if issubclass(model_cls, Model) and hasattr(model_cls, "_meta") and model_cls._meta.abstract:
modules[model_cls.__module__][model_cls.__name__] = model_cls
return modules

    @cached_property
    def model_modules(self) -> Dict[str, Dict[str, Type[Model]]]:
        """All modules that contain Django models."""
        modules: Dict[str, Dict[str, Type[Model]]] = defaultdict(dict)
        for concrete_model_cls in self.apps_registry.get_models():
            modules[concrete_model_cls.__module__][concrete_model_cls.__name__] = concrete_model_cls
            # collect abstract=True models
            for model_cls in concrete_model_cls.mro()[1:]:
                if issubclass(model_cls, Model) and hasattr(model_cls, "_meta") and model_cls._meta.abstract:
                    modules[model_cls.__module__][model_cls.__name__] = model_cls
+       print(modules)
        return modules

You could see which models django-stubs collects from the runtime.. I'm suspecting e.g. JSONFieldTestModel isn't included..

Another way might be to add in e.g. tests.sentry.db, if that's the app, to INSTALLED_APPS, run mypy again and see if any errors goes away.

@flaeppe
Copy link
Member

flaeppe commented Sep 29, 2023

Additionally I just want to point out that the real solution here is to have the plugin not add managers from the runtime model, but instead analyze the declared model(and its parents) and figure which manager attributes are implicitly added..

@asottile-sentry
Copy link
Author

was removing objects solving something specific? it seems to be breaking a lot given the pinned issue and this and I'm not sure it's worth it at this point if we can't get it correct even with runtime analysis

@flaeppe
Copy link
Member

flaeppe commented Oct 3, 2023

was removing objects solving something specific? [...]

Not more than an attempt of better alignment with the runtime..

We probably need to relax that deletion until analysis of models is more sophisticated and independent of runtime models, if that'll ever happen

asottile-sentry added a commit to getsentry/sentry-forked-django-stubs that referenced this issue Oct 31, 2023
django-stubs fails to restore these in too many cases.  see upstream issue typeddjango#1744

(not upstreamed)
asottile-sentry added a commit to getsentry/sentry-forked-django-stubs that referenced this issue Oct 31, 2023
django-stubs fails to restore these in too many cases.  see upstream issue typeddjango#1744

(not upstreamed)
asottile-sentry added a commit to getsentry/sentry-forked-django-stubs that referenced this issue Oct 31, 2023
django-stubs fails to restore these in too many cases.  see upstream issue typeddjango#1744

(not upstreamed)
asottile-sentry added a commit to getsentry/sentry-forked-django-stubs that referenced this issue Oct 31, 2023
django-stubs fails to restore these in too many cases.  see upstream issue typeddjango#1744

(not upstreamed)
asottile-sentry added a commit to getsentry/sentry-forked-django-stubs that referenced this issue Oct 31, 2023
django-stubs fails to restore these in too many cases.  see upstream issue typeddjango#1744

(not upstreamed)
asottile-sentry added a commit to getsentry/sentry-forked-django-stubs that referenced this issue Dec 5, 2023
django-stubs fails to restore these in too many cases.  see upstream issue typeddjango#1744

(not upstreamed)
asottile-sentry added a commit to getsentry/sentry-forked-django-stubs that referenced this issue Apr 30, 2024
django-stubs fails to restore these in too many cases.  see upstream issue typeddjango#1744

(not upstreamed)
asottile-sentry added a commit to getsentry/sentry-forked-django-stubs that referenced this issue May 28, 2024
django-stubs fails to restore these in too many cases.  see upstream issue typeddjango#1744

(not upstreamed)
@flaeppe flaeppe linked a pull request Jul 26, 2024 that will close this issue
asottile-sentry added a commit to getsentry/sentry-forked-django-stubs that referenced this issue Jul 29, 2024
django-stubs fails to restore these in too many cases.  see upstream issue typeddjango#1744

(not upstreamed)
@ntessman-capsule
Copy link

You could see which models django-stubs collects from the runtime.. I'm suspecting e.g. JSONFieldTestModel isn't included..

Thanks for this @flaeppe, this solved my issue! The model in question hadn't been exported properly and wasn't visible to the app config. Very oblique error message and root cause, would not have guessed the issue.

asottile-sentry added a commit to getsentry/sentry-forked-django-stubs that referenced this issue Sep 24, 2024
django-stubs fails to restore these in too many cases.  see upstream issue typeddjango#1744

(not upstreamed)
@Tatsh
Copy link
Contributor

Tatsh commented Oct 4, 2024

Confused on this issue. I've pretty much switched to ._default_manager in my projects. You can add this to the exceptions in your linter like Ruff. It's odd but it's better than the workarounds IMO.

asottile-sentry added a commit to getsentry/sentry-forked-django-stubs that referenced this issue Oct 28, 2024
django-stubs fails to restore these in too many cases.  see upstream issue typeddjango#1744

(not upstreamed)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

4 participants