Disable fallback to runtime types for Django settings

[andersk]

This fallback to value.__class__ seems to be doing more harm than good; see #312 and #1162. Replace it with a clear error message that suggests a way to fix the problem rather than incompletely papering over it.

[sobolevn]

Am I right that we should either merge this or #1162 ?

[andersk]

I think @PIG208 concluded that #1162 doesn’t really solve the problem. It happens to make my reduced test case pass, but doesn’t work on our actual large code base. And it probably regresses the type checking of request.user in some cases.

My proposal is to close #1162 and merge this, unless someone comes up with a reliable way to make the circular dependencies work. (In which case we should still merge this, since the fallback would then switch from unreliable to unnecessary.)

[sobolevn]

This feels like a safer solution to me.
So, let's remove the fallback for now.

Any other PRs that can solve the whole problem is welcome!

[andersk]

By the way, thanks for the exceptional responsiveness to our PRs. With this merged, I think we have everything we need to enable django-stubs in Zulip (zulip/zulip#18777)!

[sobolevn]

I am glad that you are happy :)
I really like reviewing PRs in TypedDjango, because they are small and clear.

It is much easier than any other project I work on.

[andersk]

What do you think about doing a PyPI release with these changes—perhaps a beta release, to give other users more opportunities to report issues with #689 and #1163?

[ljodal]

With this change we're now getting the Import cycle from Django settings module prevents type inference for 'X' [misc] error for a few of our settings. However I'm quite sure we have no import cycles in our settings file (the only imports are stdlib, external libraries, and to files that again only import from those). Any ideas what could cause this?

[sobolevn]

@ljodal if you could find a minimal reproducer - we will be glad to fix it!

[ljodal]

It seems to be happening for a random selection of our settings, from simple strings to "foo" if prod else "bar" statements, but all of these should be easy for mypy to figure out. I was hoping someone might know what could cause this, cause I'm not familiar with this part of Django stubs at all. I'm not sure if I have time to dive into it now, but I'll see. If I figure out what's going on I will of course be happy to provide a reproducer (or better a fix!) :)

[sobolevn]

Ok, let's consider this a release blocker. Something that we need to investigate before releasing our next version.

[andersk]

@ljodal My guess would be that your settings module imports something that transitively imports django.conf.settings. Although this might not be a cycle at runtime due to the laziness of Django’s settings object, it still causes a cycle at type-checking time.

(You might find a command like python -X importtime -c 'import my_project.settings' helpful for debugging.)

[ljodal]

Thanks! You are very much correct, both sentry_sdk and another internal library we use end up importing from django.conf. I assume those imports will have to be made lazily to work around this?

With regards to blocking a release I think maybe tweaking the error message a bit or adding something about this in the release notes should be enough. The official sentry docs say to set it up in settings, so I assume this might hit a few users

[andersk]

Yeah, I had the same issue with sentry_sdk. I moved it to the AppConfig ready hook (zulip/zulip@676d40d), as suggested by getsentry/sentry-docs#5326.

[ljodal]

Maybe we could add a section to the readme about this and link to that from the error message? I would assume a lot of people will end up having this issue once they upgrade. The error message pops up in kinda unrelated places, so I think it's hard to backtrack from that to the underlaying cause here.

[ljodal]

I dug a bit further and in addition to sentry_sdk, what's causing problems for us is the LANGUAGES setting where we have a list of language codes with translated names, according to the Django docs: https://docs.djangoproject.com/en/4.1/ref/settings/#languages

[PIG208]

If there is a way for us to find out the dependency cycle through mypy's API, we can provide a more informative error message. We should probably also note that the cyclic imports are not the only source of dependencies cycle for our plugin, since we inject some extra dependencies for certain modules exposed only to mypy.

[andersk]

The LANGUAGES case has a cycle of my_project.settings → django.utils.translation → django.http.request → (AUTH_USER_MODEL) → my_app.models → django.conf → my_project.settings.

I note that Django itself avoids importing django.utils.translation from django.conf.global_settings, also to avoid a dependency cycle. Perhaps the same workaround it uses would work for you:

def gettext_noop(s: str) -> str:
    return s

LANGUAGES = [
    ("de", gettext_noop("German")),
    ("en", gettext_noop("English")),
]

This will mark the strings for translation without actually translating them, which should be fine because Django itself will translate them anyway.

[ljodal]

Yeah, in our case I think I can easily work around this. That said this is a gotcha, so if possible it would be great if the error message could be a bit more helpful to point people in the right direction. At least I did not intuitively understand how my local settings file would cause an import cycle if other projects import django.conf.settings 🤔

[orsinium]

I have quite a tricky project on my hands that has the same "cyclic imports" issue (#1025 (comment)). I've untangled the following ways how settings depend on settings so far:

• my_project.__init__.py
• my_project.settings > my_project.my_app.utils > django.core.cache > django.utils.connection > django.conf

Maybe, it's possible to untangle it all, but I see it as a pretty challenging task. I'm interested in finding a way for django-stubs to get around cyclic imports rather than trying to rewrite the project for the sake of django-stubs.

Is there a way to monkey-patch django.conf.settings when importing my_project.settings from django-stubs?

[sobolevn]

You can use different DJANGO_SETTINGS_MODULE for mypy.

[orsinium]

That's a great idea! Also quite hard for this particular project, though, there are like 10k lines of spaghetti 👀 Still, I might autogenerate fake settings, maybe.

[GabDug]

Can confirm this create a lot of false positives 'Settings' object has no attribute 'XXX'. I'm not getting any Import cycle from Django settings module prevents type inference for 'XXX'.

I've tried the python -X importtime -c you suggested, but it does not seem like django.conf is imported anywhere else. I'll try a bit more or create a reproduction repo, but I think a head-ups could be added in the docs about this issue :)

Thanks to you all for your contributions !

[tony]

I am getting cyclic errors since 9bd8aed.

error: Import cycle from Django settings module prevents type inference for '...' [misc]

I'm mystified 😆

I'm grateful for the effort and the time - 🙏. Can I ask you make a second PR with tests, documentation, and migration notes?

[intgr]

Ok, let's consider this a release blocker. Something that we need to investigate before releasing our next version.

Oof.

Apparently this continues to be problematic for users, please open an issue for discussion about possible solutions.

[orsinium]

please open an issue for discussion about possible solutions.

@intgr you can take the initiative on that, why not. Looks like you have a sample code of when it goes wrong, that would be a great discussion starter.

[intgr]

Nope, this is the first time I heard of this issue. I have no sample code.

[jgillard]

I've not got a minimal reproduction, but I followed the advice above to track down this issue to sentry_sdk, but not import sentry_sdk as that works fine, only from sentry_sdk.integrations.django import DjangoIntegration causes this issue for me.

[andersk]

@jgillard As per above, moving your sentry_sdk.init call to the AppConfig.ready hook will fix that.

[jgillard]

OK. I was wondering if another change was due for this issue, as it's not obvious that this is the suggested solution without reading this thread. So the hope is that the sentry-docs suggestion gets merged then.

[flaeppe]

Revisiting this I realised that whenever one has explicit settings declared in a settings.py file and getting import cycle errors;

error: Import cycle from Django settings module prevents type inference for 'SOME_SETTING'  [misc]

It was possible to get rid of it by explicitly annotate the setting variable instead of having mypy figure it out by itself

...
SOME_SETTING: int = func_returning_int()
...

[orsinium]

Oh my, it works! Thank you!

[intgr]

Created a Discussion along with new category "Common issues and solutions"

• Fixing 'Import cycle from Django settings module prevents type inference' #2097