Avoid importing django.test package when not testing
#8699
Conversation
rest_framework/settings.py
Outdated
| try: | ||
| from django.core.signals import setting_changed | ||
| except ImportError: | ||
| from django.test.signals import setting_changed |
There was a problem hiding this comment.
Can't we always just import it from django.core.signals?
It looks to me like that's where it's defined... https://github.com/django/django/blob/main/django/test/signals.py#L9
It what cases is the ImportError raised?
There was a problem hiding this comment.
The signal import path is documented as django.test.signals.setting_changed. So technically the django.core.signals is an implementation detail that is not guaranteed to be stable. So I added the fallback only for future compat. That said, I don't foresee it changing, and even if it does change, it can be fixed in a new release.
Should I remove the fallback?
There was a problem hiding this comment.
🤷🏼♂️ Seems low risk to me.
Importing anything `rest_framework` causes `django.test` to be imported. This is because DRF registers a receiver on the `django.test_signals.setting_changed` signal. This is not really a problem, but it is good to avoid this because it bloats the memory with unnecessary modules (e.g. `django.test`, `django.core.servers.basehttp`, `socketserver`) and increases the startup time. It also doesn't feel right to import test code into non-test code. Try to import the signal from a core module if possible. Note that there's another `django.test` import in `MultiPartRenderer`, however this import is done lazily only if the functionality is used so can be easily avoided.
bluetech
force-pushed
the
avoid-test-import
branch
from
4dd40fc to
235f7e1
Compare
|
Let's 'av it. |
2 participants
Description
Importing anything from
rest_frameworkcausesdjango.testto be imported. This is because DRF registers a receiver on thedjango.test_signals.setting_changedsignal.This is not really a problem, but it is good to avoid this because it bloats the memory with unnecessary modules (e.g.
django.test,django.core.servers.basehttp,socketserver) and increases the startup time. It also doesn't feel right to import test code into non-test code.Try to import the signal from a core module if possible.
Note that there's another
django.testimport inMultiPartRenderer, however this import is done lazily only if the functionality is used so can be easily avoided.