id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
36864	Automatic shell imports cannot reliably handle submodules	Jacob Walls	Jacob Walls	"Adjust the test for customizing automatic shell imports to include a submodule of a module not normally imported, like `django.views.decorators`:

{{{#!py
diff --git a/tests/shell/tests.py b/tests/shell/tests.py
index 78e61e6a9d..95041e455e 100644
--- a/tests/shell/tests.py
+++ b/tests/shell/tests.py
@@ -366,7 +366,7 @@ class ShellCommandAutoImportsTestCase(SimpleTestCase):
     def test_message_with_stdout_one_object(self):
         class TestCommand(shell.Command):
             def get_auto_imports(self):
-                return [""django.db.connection""]
+                return [""django.views.decorators.gzip""]
 
         with captured_stdout() as stdout:
             TestCommand().get_namespace(verbosity=2)
@@ -376,7 +376,7 @@ class ShellCommandAutoImportsTestCase(SimpleTestCase):
             1: ""1 object imported automatically (use -v 2 for details)."",
             2: (
                 ""1 object imported automatically:\n\n""
-                ""  from django.db import connection""
+                ""  from django.views.decorators import gzip""
             ),
         }
         for verbosity, expected in cases.items():
}}}

This fails for the reason developed in #35402 that `import_string()` has trouble with modules: top-level modules always fail, and submodules sometimes succeed by happenstance. Here, `django.db.connection` only works as an automatic import because we depend on coincidentally importing `django.db` when running management commands (by importing `django.core.checks`). This seems fragile.

{{{#!py
AssertionError: '1 object could not be automatically imported:\n\n [62 chars]lly.' != '1 object imported automatically:\n\n  from django.[24 chars]gzip'
- 1 object could not be automatically imported:
+ 1 object imported automatically:
  
-   django.views.decorators.gzip
?                          ^
+   from django.views.decorators import gzip
?  +++++                        ^^^^^^^^
- 
- 0 objects imported automatically.
}}}

If you're interested to see why:
{{{#!py
>>> import django
>>> getattr(django, ""db"")  # fails
AttributeError ...
>>> import django.db  # coincidental import somewhere
>>> getattr(django, ""db"")  # succeeds
<module 'django.db' ...
}}}

----
`import_string()` is [https://docs.djangoproject.com/en/6.0/ref/utils/#django.utils.module_loading.import_string documented] to ""return the attribute/class designated by the last name in the path"". I guess a submodule is an ""attribute"" of a parent module, so it's suggested that this works, but looking at existing usage I found that often the assumption is that only classes and callables succeed. (I added tests that fail if this assumption is changed in b99c608ea10cabc97a6b251cdb6e81ef2a83bdcf.)


I'm suggesting we make `import_string()` fail more consistently when given a submodule, to surface these sneaky problems (after a deprecation).

----
Or, we could make `import_string()` handle either submodules or top-level modules as well.

Tim Graham [https://github.com/django/django/pull/18103#issuecomment-2135493131 observed that] werkzeug's implementation of `import_string()` handles both modules and submodules:
{{{#!py
        except ImportError:
            # support importing modules not yet set up by the parent module
            # (or package for that matter)
            module = import_string(module_name)
}}}

But that's not the history we have for this util. At the time, I thought it would be a bigger lift to audit all the call sites of `import_string()` and make sure they're OK with modules & submodules now succeeding. We also have several places coping with the ambiguity of modules and classes correctly:

{{{#!py
        # If import_module succeeds, entry points to the app module.
        try:
            app_module = import_module(entry)
...
        # If import_string succeeds, entry is an app config class.
        if app_config_class is None:
            try:
                app_config_class = import_string(entry)
...
        # If both import_module and import_string failed, it means that entry
        # doesn't have a valid value.
}}}

...but I'm alright with that assumption being questioned. I just think we're on the hook here for one behavior change or the other, since now it affects automatic shell imports.

PR incoming for the ""reliably raise"" option to aid the triage."	Bug	assigned	Core (Management commands)	5.2	Normal		import_string, submodule		Accepted	0	0	0	0	0	0