From 537c6df3e641ebc21e9da76c86aea4d36e6a32e2 Mon Sep 17 00:00:00 2001 From: Ben Cail Date: Tue, 1 Nov 2022 14:55:40 -0400 Subject: [PATCH 1/3] Fixed #32915 -- Suppressed fewer ImportErrors in management commands. Before, exceptions during settings loading were swallowed by management commands. Now, if the new command attribute settings_required is True (default), the error will be printed to stderr, and the command will exit non-zero. makemessages, runserver, shell, and startproject continue to tolerate missing settings (via settings_required=False). Regression in c6864a01b25591d3a709da8071413b69c9e35341, which added support for some commands (e.g. startproject) to cope with missing settings. Thanks Mariusz Felisiak, Sarah Boyce, Jacob Walls, Mike Edmunds, and Brian Helba for reviews. Co-authored-by: Rohith PR --- django/conf/__init__.py | 9 +++++- django/core/management/__init__.py | 21 ++++++------ django/core/management/base.py | 5 +++ .../core/management/commands/makemessages.py | 1 + django/core/management/commands/shell.py | 1 + django/core/management/commands/startapp.py | 1 + .../core/management/commands/startproject.py | 1 + docs/howto/custom-management-commands.txt | 7 ++++ docs/releases/6.2.txt | 4 +++ tests/admin_scripts/tests.py | 32 ++++++++++++++++--- tests/settings_tests/tests.py | 11 +++++++ 11 files changed, 76 insertions(+), 17 deletions(-) diff --git a/django/conf/__init__.py b/django/conf/__init__.py index b3daea5b7b9f..7a9ed9418b1f 100644 --- a/django/conf/__init__.py +++ b/django/conf/__init__.py @@ -248,7 +248,14 @@ def __init__(self, settings_module): # store the settings module in case someone later cares self.SETTINGS_MODULE = settings_module - mod = importlib.import_module(self.SETTINGS_MODULE) + try: + mod = importlib.import_module(self.SETTINGS_MODULE) + except ImportError as exc: + # If settings cannot be imported, treat as a configuration error. + if exc.name == self.SETTINGS_MODULE: + msg = f"No module named {self.SETTINGS_MODULE!r}." + raise ImproperlyConfigured(msg) from exc + raise tuple_settings = ( "ALLOWED_HOSTS", diff --git a/django/core/management/__init__.py b/django/core/management/__init__.py index f547ef730cd7..be234906718c 100644 --- a/django/core/management/__init__.py +++ b/django/core/management/__init__.py @@ -206,6 +206,7 @@ def __init__(self, argv=None): if self.prog_name == "__main__.py": self.prog_name = "python -m django" self.settings_exception = None + self.style = color_style() def main_help_text(self, commands_only=False): """Return the script's main help text, as a string.""" @@ -226,16 +227,15 @@ def main_help_text(self, commands_only=False): else: app = app.rpartition(".")[-1] commands_dict[app].append(name) - style = color_style() for app in sorted(commands_dict): usage.append("") - usage.append(style.NOTICE("[%s]" % app)) + usage.append(self.style.NOTICE("[%s]" % app)) for name in sorted(commands_dict[app]): usage.append(" %s" % name) # Output an extra note if settings are not properly configured if self.settings_exception is not None: usage.append( - style.NOTICE( + self.style.NOTICE( "Note that only Django core commands are listed " "as settings are not properly configured (error: %s)." % self.settings_exception @@ -255,14 +255,10 @@ def fetch_command(self, subcommand): try: app_name = commands[subcommand] except KeyError: - if os.environ.get("DJANGO_SETTINGS_MODULE"): - # If `subcommand` is missing due to misconfigured settings, the - # following line will retrigger an ImproperlyConfigured - # exception (get_commands() swallows the original one) so the - # user is informed about it. - settings.INSTALLED_APPS + if os.environ.get("DJANGO_SETTINGS_MODULE") and self.settings_exception: + sys.stderr.write(self.style.NOTICE(str(self.settings_exception) + "\n")) elif not settings.configured: - sys.stderr.write("No Django settings specified.\n") + sys.stderr.write(self.style.NOTICE("No Django settings specified.\n")) possible_matches = get_close_matches(subcommand, commands) sys.stderr.write("Unknown command: %r" % subcommand) if possible_matches: @@ -274,6 +270,9 @@ def fetch_command(self, subcommand): klass = app_name else: klass = load_command_class(app_name, subcommand) + if self.settings_exception and klass.requires_settings: + sys.stderr.write(self.style.NOTICE(str(self.settings_exception) + "\n")) + sys.exit(1) return klass def autocomplete(self): @@ -383,8 +382,6 @@ def execute(self): settings.INSTALLED_APPS except ImproperlyConfigured as exc: self.settings_exception = exc - except ImportError as exc: - self.settings_exception = exc if settings.configured: # Start the auto-reloading dev server even if the code is broken. diff --git a/django/core/management/base.py b/django/core/management/base.py index 8f2447905064..983e24c0ba44 100644 --- a/django/core/management/base.py +++ b/django/core/management/base.py @@ -245,6 +245,10 @@ class BaseCommand: A boolean; if ``True``, the command prints a warning if the set of migrations on disk don't match the migrations in the database. + ``requires_settings`` + A boolean; if ``False``, an ``ImportError`` from a missing settings + module is suppressed. + ``requires_system_checks`` A list or tuple of tags, e.g. [Tags.staticfiles, Tags.models]. System checks registered in the chosen tags will be checked for errors prior @@ -269,6 +273,7 @@ class BaseCommand: _called_from_command_line = False output_transaction = False # Whether to wrap the output in a "BEGIN; COMMIT;" requires_migrations_checks = False + requires_settings = True requires_system_checks = "__all__" # Arguments, common to all commands, which aren't defined by the argument # parser. diff --git a/django/core/management/commands/makemessages.py b/django/core/management/commands/makemessages.py index bf4ce5fc445c..5a4bd55bb8be 100644 --- a/django/core/management/commands/makemessages.py +++ b/django/core/management/commands/makemessages.py @@ -208,6 +208,7 @@ class Command(BaseCommand): translatable_file_class = TranslatableFile build_file_class = BuildFile + requires_settings = False requires_system_checks = [] msgmerge_options = ["-q", "--backup=none", "--previous", "--update"] diff --git a/django/core/management/commands/shell.py b/django/core/management/commands/shell.py index 47342291e8f0..8366e2872247 100644 --- a/django/core/management/commands/shell.py +++ b/django/core/management/commands/shell.py @@ -19,6 +19,7 @@ class Command(BaseCommand): "as code." ) + requires_settings = False requires_system_checks = [] shells = ["ipython", "bpython", "python"] diff --git a/django/core/management/commands/startapp.py b/django/core/management/commands/startapp.py index e85833b9a827..e722056d7756 100644 --- a/django/core/management/commands/startapp.py +++ b/django/core/management/commands/startapp.py @@ -7,6 +7,7 @@ class Command(TemplateCommand): "the current directory or optionally in the given directory." ) missing_args_message = "You must provide an application name." + requires_settings = False def handle(self, **options): app_name = options.pop("name") diff --git a/django/core/management/commands/startproject.py b/django/core/management/commands/startproject.py index 2b4f953e366f..6c1d8873a82c 100644 --- a/django/core/management/commands/startproject.py +++ b/django/core/management/commands/startproject.py @@ -9,6 +9,7 @@ class Command(TemplateCommand): "name in the current directory or optionally in the given directory." ) missing_args_message = "You must provide a project name." + requires_settings = False def handle(self, **options): project_name = options.pop("name") diff --git a/docs/howto/custom-management-commands.txt b/docs/howto/custom-management-commands.txt index dddfb5b19b4d..7b9b4e26fef2 100644 --- a/docs/howto/custom-management-commands.txt +++ b/docs/howto/custom-management-commands.txt @@ -222,6 +222,13 @@ All attributes can be set in your derived class and can be used in migrations on disk don't match the migrations in the database. A warning doesn't prevent the command from executing. Default value is ``False``. +.. attribute:: BaseCommand.requires_settings + + .. versionadded:: 6.2 + + A boolean; if ``False``, any :exc:`ImportError` from a missing settings + module is suppressed. Default value is ``True``. + .. attribute:: BaseCommand.requires_system_checks A list or tuple of tags, e.g. ``[Tags.staticfiles, Tags.models]``. System diff --git a/docs/releases/6.2.txt b/docs/releases/6.2.txt index c2c8eed5e8b8..d40237d86f32 100644 --- a/docs/releases/6.2.txt +++ b/docs/releases/6.2.txt @@ -183,6 +183,10 @@ Management Commands model renames. After upgrading, you will see new migrations detected for unmanaged models that have changed since their creation. +* Whether to suppress an :exc:`ImportError` escaping from a settings module is + configurable by the new :attr:`.BaseCommand.requires_settings` attribute + (default ``True``). In previous versions, such errors were always suppressed. + Migrations ~~~~~~~~~~ diff --git a/tests/admin_scripts/tests.py b/tests/admin_scripts/tests.py index 3eb7b97c99ce..3178cdf4df97 100644 --- a/tests/admin_scripts/tests.py +++ b/tests/admin_scripts/tests.py @@ -29,6 +29,8 @@ call_command, color, execute_from_command_line, + get_commands, + load_command_class, ) from django.core.management.base import LabelCommand, SystemCheckError from django.core.management.commands.listurls import Command as ListurlsCommand @@ -257,10 +259,19 @@ def test_commands_with_invalid_settings(self): Commands that don't require settings succeed if the settings file doesn't exist. """ - args = ["startproject"] - out, err = self.run_django_admin(args, settings_file="bad_settings") - self.assertNoOutput(out) - self.assertOutput(err, "You must provide a project name", regex=True) + for cmd, owner in get_commands().items(): + if owner != "django.core": + continue + with self.subTest(command=cmd): + out, err = self.run_django_admin( + [cmd, "--help"], settings_file="bad_settings" + ) + klass = load_command_class(owner, cmd) + if klass.requires_settings: + msg = "No module named 'bad_settings'." + self.assertOutput(err, msg) + else: + self.assertNotIn("bad_settings", err) class DjangoAdminDefaultSettings(AdminScriptTestCase): @@ -889,6 +900,18 @@ def test_builtin_with_bad_settings(self): self.assertNoOutput(out) self.assertOutput(err, "No module named '?bad_settings'?", regex=True) + def test_runserver_with_bad_settings(self): + args = ["runserver", "--settings=bad_settings", "--nostatic"] + out, err = self.run_manage(args) + self.assertNoOutput(out) + self.assertOutput(err, "No module named '?bad_settings'?", regex=True) + + def test_startapp_with_bad_settings(self): + args = ["startapp", "--settings=bad_settings", "app1"] + out, err = self.run_manage(args) + self.assertNoOutput(out) + self.assertNoOutput(err) + def test_builtin_with_bad_environment(self): """ no settings: manage.py builtin commands fail if settings file (from @@ -1867,6 +1890,7 @@ def test_empty_allowed_hosts_error(self): class ManageRunserverHelpOutput(AdminScriptTestCase): def test_suppressed_options(self): """runserver doesn't support --verbosity and --trackback options.""" + self.write_settings("settings.py") out, err = self.run_manage(["runserver", "--help"]) self.assertNotInOutput(out, "--verbosity") self.assertNotInOutput(out, "--trackback") diff --git a/tests/settings_tests/tests.py b/tests/settings_tests/tests.py index 7794aeef9a36..3c52f31df8c0 100644 --- a/tests/settings_tests/tests.py +++ b/tests/settings_tests/tests.py @@ -339,6 +339,17 @@ def test_incorrect_timezone(self): with self.assertRaisesMessage(ValueError, "Incorrect timezone setting: test"): settings._setup() + def test_unable_to_import_settings_module(self): + msg = "No module named 'fake_settings_module'." + with self.assertRaisesMessage(ImproperlyConfigured, msg): + Settings("fake_settings_module") + + def test_unable_to_import_a_random_module(self): + exc = ModuleNotFoundError("No module named 'fake_module'", name="fake_module") + with mock.patch("importlib.import_module", side_effect=exc): + with self.assertRaisesMessage(ImportError, "No module named 'fake_module'"): + Settings("fake_settings_module") + class TestComplexSettingOverride(SimpleTestCase): def setUp(self): From ad289c1de4c39cc2b35748dc7a5ac84c7cbdf10a Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Thu, 16 Jul 2026 12:00:28 -0400 Subject: [PATCH 2/3] Fixed #37218 -- Doc'd that on-demand fetching is sync only. --- docs/topics/db/queries.txt | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/docs/topics/db/queries.txt b/docs/topics/db/queries.txt index cbfa3c1949e0..e5a09a3a4e65 100644 --- a/docs/topics/db/queries.txt +++ b/docs/topics/db/queries.txt @@ -996,8 +996,10 @@ In there, you'll find the methods on QuerySets grouped into two sections: * *Methods that return new querysets*: These are the non-blocking ones, and don't have asynchronous versions. You're free to use these in any - situation, though read the notes on ``defer()`` and ``only()`` before you use - them. + situation, but understand the constraint of `no deferred queries`_ before + using :meth:`~django.db.models.query.QuerySet.defer`, + :meth:`~django.db.models.query.QuerySet.only`, or + :meth:`~django.db.models.query.QuerySet.fetch_mode`. * *Methods that do not return querysets*: These are the blocking ones, and have asynchronous versions - the asynchronous name for each is noted in its @@ -1020,6 +1022,15 @@ the whole expression in order to call it in an asynchronous-friendly way. place of your model instances. If you ever see these, you are missing an ``await`` somewhere to turn that coroutine into a real value. +No deferred queries +------------------- + +Relations are deferred by default. If the ``blog`` relation is not prefetched, +accessing ``entry.blog`` during asynchronous evaluation of a queryset attempts +a blocking database query, raising ``SynchronousOnlyOperation``. (A similar +limitation applies to fields deferred via ``defer()`` or ``only()``.) To avoid +this, see the advice on prefetching in :ref:`topics-db-queries-related`. + Transactions ------------ From 274df4df0bca7fcfb5c1c1d49567f770df147eeb Mon Sep 17 00:00:00 2001 From: Stephanie <104656459+StephanieAG@users.noreply.github.com> Date: Sun, 28 Jun 2026 09:40:00 -0400 Subject: [PATCH 3/3] Fixed #37093 -- Clarified pull request instructions and adjusted error messages. The intention is to reduce the perceived harshness of error messages so that they look more like just a friendly list of things in your PR to fix, especially for new contributors. --- .github/pull_request_template.md | 9 ++++--- scripts/pr_quality/check_pr.py | 13 ++-------- scripts/pr_quality/errors.py | 6 ++--- scripts/pr_quality/tests/test_check_pr.py | 30 ++++++----------------- 4 files changed, 17 insertions(+), 41 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 446c6b37e3ef..d3a9d64ec1a8 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -5,19 +5,22 @@ ticket-XXXXX #### Branch description -Provide a concise overview of the issue or rationale behind the proposed changes. + #### AI Assistance Disclosure (REQUIRED) - + - [ ] **No AI tools were used** in preparing this PR. - [ ] **If AI tools were used**, I have disclosed which ones, and fully reviewed and verified their output. + #### Checklist - [ ] This PR follows the [contribution guidelines](https://docs.djangoproject.com/en/stable/internals/contributing/writing-code/submitting-patches/). - [ ] This PR **does not** disclose a security vulnerability (see [vulnerability reporting](https://docs.djangoproject.com/en/stable/internals/security/)). - [ ] This PR targets the `main` branch. -- [ ] The commit message is written in past tense, mentions the ticket number, and ends with a period (see [guidelines](https://docs.djangoproject.com/en/dev/internals/contributing/committing-code/#committing-guidelines)). +- [ ] The commit message is written in past tense, mentions the ticket number (if applicable), and ends with a period (see [guidelines](https://docs.djangoproject.com/en/dev/internals/contributing/committing-code/#committing-guidelines)). - [ ] I have not requested, and will not request, an automated AI review for this PR. + + - [ ] I have checked the "Has patch" ticket flag in the Trac system. - [ ] I have added or updated relevant tests. - [ ] I have added or updated relevant docs, including release notes if applicable. diff --git a/scripts/pr_quality/check_pr.py b/scripts/pr_quality/check_pr.py index 2cf225a27aac..272e003567d8 100644 --- a/scripts/pr_quality/check_pr.py +++ b/scripts/pr_quality/check_pr.py @@ -330,16 +330,7 @@ def check_pr_title_has_ticket(pr_title, ticket_id): def check_branch_description(pr_body): - """The branch description must be present. - - The description should not contain the placeholder, and should be at least - 5 words long. - """ - placeholder = ( - "Provide a concise overview of the issue or rationale behind the" - " proposed changes." - ) - + """The branch description should be at least 5 words long.""" description_match = re.search( r"#### Branch description[ \t]*\r?\n(.*?)(?=\r?\n####|\Z)", pr_body, @@ -351,7 +342,7 @@ def check_branch_description(pr_body): # Strip HTML comments before evaluating content. cleaned = strip_html_comments(description_match.group(1)).strip() - if not cleaned or placeholder in cleaned or len(cleaned.split()) < MIN_WORDS: + if not cleaned or len(cleaned.split()) < MIN_WORDS: return Message(*MISSING_DESCRIPTION) return None diff --git a/scripts/pr_quality/errors.py b/scripts/pr_quality/errors.py index cd41612da341..eb75b556f5f8 100644 --- a/scripts/pr_quality/errors.py +++ b/scripts/pr_quality/errors.py @@ -5,7 +5,7 @@ """ -LEVEL_ERROR = ("🛑", "Error") +LEVEL_ERROR = ("❗", "Error") LEVEL_WARNING = ("⚠️", "Warning") @@ -112,9 +112,7 @@ def as_details(self, level=LEVEL_ERROR): MISSING_DESCRIPTION = ( "Missing PR Description", - "Your PR description must be substantive and meaningful. The placeholder text " - '"*Provide a concise overview of the issue or rationale behind the proposed ' - 'changes.*" is not acceptable.\n\n' + "Your PR description must be substantive and meaningful.\n\n" "**What to do:**\n\n" "Write a description that contains at least 5 words and addresses:\n\n" "- What problem does this PR solve?\n" diff --git a/scripts/pr_quality/tests/test_check_pr.py b/scripts/pr_quality/tests/test_check_pr.py index 0aff15ca109b..74b33500001f 100644 --- a/scripts/pr_quality/tests/test_check_pr.py +++ b/scripts/pr_quality/tests/test_check_pr.py @@ -43,12 +43,13 @@ def make_pr_body( "This PR targets the `main` branch." " ", "The commit message is written in past tense, mentions the ticket" - " number, and ends with a period (see [guidelines]" + " number (if applicable), and ends with a period (see [guidelines]" "(https://docs.djangoproject.com/en/dev/internals/contributing/" "committing-code/#committing-guidelines)).", "I have not requested, and will not request, an automated AI review" " for this PR." - " ", + " \n\n" + "", 'I have checked the "Has patch" ticket flag in the Trac system.', "I have added or updated relevant tests.", "I have added or updated relevant docs, including release notes if" @@ -73,10 +74,11 @@ def make_pr_body( f"{description}\n" f"\n" f"#### AI Assistance Disclosure (REQUIRED)\n" - f"\n" + f"\n" f"- {no_ai_box} **No AI tools were used** in preparing this PR.\n" f"- {ai_used_box} **If AI tools were used**, I have disclosed which" f" ones, and fully reviewed and verified their output.{ai_extra}\n" + f"\n" f"\n" f"#### Checklist\n" f"{checklist_lines}\n" @@ -465,24 +467,6 @@ class TestCheckBranchDescription(BaseTestCase): def test_valid_passes(self): self.assertIsNone(check_pr.check_branch_description(VALID_PR_BODY)) - def test_placeholder_fails(self): - body = make_pr_body( - description=( - "Provide a concise overview of the issue or rationale behind" - " the proposed changes." - ) - ) - self.assertIsNotNone(check_pr.check_branch_description(body)) - - def test_placeholder_with_appended_text_fails(self): - body = make_pr_body( - description=( - "Provide a concise overview of the issue or rationale behind" - " the proposed changes. Yes." - ) - ) - self.assertIsNotNone(check_pr.check_branch_description(body)) - def test_empty_fails(self): body = make_pr_body(description="") self.assertIsNotNone(check_pr.check_branch_description(body)) @@ -688,8 +672,8 @@ def test_make_pr_body_matches_template(self): blank_body = make_pr_body( ticket="ticket-XXXXX", description=( - "Provide a concise overview of the issue or rationale behind" - " the proposed changes." + "" ), no_ai_checked=False, ai_used_checked=False,