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/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/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 ------------ 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, 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):