chore: enable core Ruff rule groups (#286)

Author: joshuadavidthomas

noxfile.py

@@ -47,11 +47,8 @@ def should_skip(python: str, django: str) -> bool:
         # Django 5.2 requires Python 3.10+
         return True
 
-    if django == DJ51 and version(python) < version(PY310):
-        # Django 5.1 requires Python 3.10+
-        return True
-
-    return False
+    # Django 5.1 requires Python 3.10+
+    return django == DJ51 and version(python) < version(PY310)
 
 
 @nox.session

pyproject.toml

@@ -174,8 +174,8 @@ extend-include = ["*.pyi?"]
 indent-width = 4
 # Same as Black.
 line-length = 88
-# Assume Python >3.8
-target-version = "py38"
+# Same as requires-python.
+target-version = "py39"
 
 [tool.ruff.format]
 # Like Black, indent with spaces, rather than tabs.
@@ -192,10 +192,23 @@ dummy-variable-rgx = "^(_+|(_+[a-zA-Z0-9_]*[a-zA-Z0-9]+?))$"
 fixable = ["A", "B", "C", "D", "E", "F", "I"]
 ignore = ["E501", "E741"]  # temporary
 select = [
+  "A",  # flake8-builtins
   "B",  # flake8-bugbear
-  "E",  # Pycodestyle
+  "C4",  # flake8-comprehensions
+  "DJ",  # flake8-django
+  "DTZ",  # flake8-datetimez
+  "E",  # pycodestyle
   "F",  # Pyflakes
+  "G",  # flake8-logging-format
   "I",  # isort
+  "LOG",  # flake8-logging
+  "PIE",  # flake8-pie
+  "PT",  # flake8-pytest-style
+  "RET",  # flake8-return
+  "RUF100",  # unused noqa
+  "S",  # flake8-bandit
+  "SIM",  # flake8-simplify
+  "T20",  # flake8-print
   "UP"  # pyupgrade
 ]
 unfixable = []
@@ -206,6 +219,10 @@ known-first-party = ["email_relay"]
 required-imports = ["from __future__ import annotations"]
 
 [tool.ruff.lint.per-file-ignores]
+# Release helper intentionally shells out to Git/GitHub CLI and prints progress.
+".bin/bump.py" = ["PIE810", "S110", "S602", "T201"]
+# Sphinx uses this conventional variable name.
+"docs/conf.py" = ["A001"]
 # Tests can use magic values, assertions, and relative imports
 "tests/**/*" = ["PLR2004", "S101", "TID252"]
 

src/email_relay/management/commands/runrelay.py

@@ -58,7 +58,7 @@ def delete_old_messages(self) -> None:
                         seconds=app_settings.MESSAGES_RETENTION_SECONDS
                     )
                 )
-            logger.debug(f"deleted {deleted_messages} messages")
+            logger.debug("deleted %s messages", deleted_messages)
 
     def ping_healthcheck(self) -> None:
         if app_settings.RELAY_HEALTHCHECK_URL is not None:
@@ -77,12 +77,14 @@ def ping_healthcheck(self) -> None:
                     timeout=app_settings.RELAY_HEALTHCHECK_TIMEOUT,
                 )
             except requests.exceptions.RequestException as e:
-                logger.warning(f"healthcheck failed, got exception: {e}")
+                logger.warning("healthcheck failed, got exception: %s", e)
                 return
 
             if response.status_code == app_settings.RELAY_HEALTHCHECK_STATUS_CODE:
                 logger.debug("healthcheck ping successful")
             else:
                 logger.warning(
-                    f"healthcheck failed, got {response.status_code}, expected {app_settings.RELAY_HEALTHCHECK_STATUS_CODE}"
+                    "healthcheck failed, got %s, expected %s",
+                    response.status_code,
+                    app_settings.RELAY_HEALTHCHECK_STATUS_CODE,
                 )

src/email_relay/models.py

@@ -36,7 +36,7 @@ def get_message_batch(self) -> list[Message]:
                 self.deferred().prioritized(),  # type: ignore[attr-defined]
             )
         )
-        logger.debug(f"found {len(message_batch)} messages to send")
+        logger.debug("found %s messages to send", len(message_batch))
         if app_settings.EMAIL_MAX_BATCH is not None:
             msg = f"max batch size is {app_settings.EMAIL_MAX_BATCH}"
             if len(message_batch) > app_settings.EMAIL_MAX_BATCH:
@@ -45,8 +45,8 @@ def get_message_batch(self) -> list[Message]:
             message_batch = message_batch[: app_settings.EMAIL_MAX_BATCH]
         return message_batch
 
-    def get_message_for_sending(self, id: int) -> Message:
-        return self.filter(id=id).select_for_update(skip_locked=True).get()
+    def get_message_for_sending(self, message_id: int) -> Message:
+        return self.filter(id=message_id).select_for_update(skip_locked=True).get()
 
     def messages_available_to_send(self) -> bool:
         return self.queued().exists() or self.deferred().exists()  # type: ignore[attr-defined]
@@ -124,7 +124,7 @@ def save(self, *args, **kwargs):
         # Overriding the save method in order to make sure that
         # modified field is updated even if it is not given as
         # a parameter to the update field argument.
-        update_fields = kwargs.get("update_fields", None)
+        update_fields = kwargs.get("update_fields")
         if update_fields:
             kwargs["update_fields"] = set(update_fields).union({"updated_at"})
 

src/email_relay/relay.py

@@ -45,7 +45,7 @@ def send_all():
                 if email is not None:
                     email.connection = connection
                     email.send()
-                    logger.debug(f"sent message {message.id}")
+                    logger.debug("sent message %s", message.id)
                     message.mark_sent()
                     counts["sent"] += 1
                 else:
@@ -65,23 +65,23 @@ def send_all():
                     and message.retry_count >= app_settings.EMAIL_MAX_RETRIES
                 ):
                     logger.warning(
-                        f"max retries reached, marking message {message.id} as failed"
+                        "max retries reached, marking message %s as failed", message.id
                     )
                     message.fail(log=str(err))
                     connection = None
                     counts["failed"] += 1
                     continue
 
                 logger.debug(
-                    f"deferring message {message.id} due to {err}", exc_info=True
+                    "deferring message %s due to %s", message.id, err, exc_info=True
                 )
                 message.defer(log=str(err))
                 connection = None
                 counts["deferred"] += 1
             except Exception as err:
-                logger.error(
-                    f"unexpected error processing message {message.id}, marking as failed.",
-                    exc_info=True,
+                logger.exception(
+                    "unexpected error processing message %s, marking as failed.",
+                    message.id,
                 )
                 message.fail(log=str(err))
                 connection = None
@@ -92,16 +92,21 @@ def send_all():
             and counts["deferred"] >= app_settings.EMAIL_MAX_DEFERRED
         ):
             logger.debug(
-                f"max deferred emails reached ({app_settings.EMAIL_MAX_DEFERRED}), stopping"
+                "max deferred emails reached (%s), stopping",
+                app_settings.EMAIL_MAX_DEFERRED,
             )
             break
 
         if app_settings.EMAIL_THROTTLE > 0:
             logger.debug(
-                f"throttling enabled, sleeping for {app_settings.EMAIL_THROTTLE} seconds"
+                "throttling enabled, sleeping for %s seconds",
+                app_settings.EMAIL_THROTTLE,
             )
             time.sleep(app_settings.EMAIL_THROTTLE)
 
     logger.info(
-        f"sent {counts['sent']} emails, deferred {counts['deferred']} emails, failed {counts['failed']} emails"
+        "sent %s emails, deferred %s emails, failed %s emails",
+        counts["sent"],
+        counts["deferred"],
+        counts["failed"],
     )

src/email_relay/service.py

@@ -26,7 +26,7 @@ def get_user_settings_from_env() -> dict[str, Any]:
         >>> get_user_settings_from_env()
         {'DEBUG': False}
     """
-    all_env_vars = {k: v for k, v in os.environ.items()}
+    all_env_vars = dict(os.environ)
     env_vars_dict = env_vars_to_nested_dict(all_env_vars)
     valid_settings = filter_valid_django_settings(env_vars_dict)
     return coerce_dict_values(valid_settings)
@@ -176,7 +176,7 @@ def run_relay_service() -> int:
     settings.configure(**SETTINGS)
     django.setup()
     call_command("migrate")
-    print("Starting email relay service...")
+    print("Starting email relay service...")  # noqa: T201
     call_command("runrelay")
     # should never get here, `runrelay` is an infinite loop
     # but if it does, exit with 0

tests/test_conf.py

@@ -8,7 +8,7 @@
 
 
 @pytest.mark.parametrize(
-    "setting,default_setting",
+    ("setting", "default_setting"),
     [
         ("DATABASE_ALIAS", "email_relay_db"),
         ("EMAIL_MAX_BATCH", None),
@@ -32,7 +32,7 @@ def test_default_settings(setting, default_setting):
 
 
 @pytest.mark.parametrize(
-    "setting,user_setting",
+    ("setting", "user_setting"),
     [
         ("DATABASE_ALIAS", "custom_db_name"),
         ("EMAIL_MAX_BATCH", 10),

tests/test_email.py

@@ -78,7 +78,7 @@ def test_to_dict():
     email_dict = RelayEmailData.from_email_message(email_message).to_dict()
 
     assert email_dict == IsPartialDict(
-        **{
+        {
             "subject": email_message.subject,
             "body": email_message.body,
             "from_email": email_message.from_email,
@@ -111,7 +111,7 @@ def test_to_dict_multi_alternatives():
     email_dict = RelayEmailData.from_email_message(email_multi_alternatives).to_dict()
 
     assert email_dict == IsPartialDict(
-        **{
+        {
             "subject": email_multi_alternatives.subject,
             "body": email_multi_alternatives.body,
             "from_email": email_multi_alternatives.from_email,
@@ -147,7 +147,7 @@ def test_to_dict_with_attachment():
     email_dict = RelayEmailData.from_email_message(email).to_dict()
 
     assert email_dict == IsPartialDict(
-        **{
+        {
             "subject": email.subject,
             "body": email.body,
             "from_email": email.from_email,

tests/test_models.py

@@ -53,7 +53,7 @@ def test_get_message_for_sending(self):
         assert message_for_sending == message
 
     @pytest.mark.parametrize(
-        "status, expected",
+        ("status", "expected"),
         [
             (Status.QUEUED, True),
             (Status.DEFERRED, True),

tests/test_runrelay.py

@@ -40,7 +40,7 @@ def test_command_with_empty_queue(runrelay, mailoutbox):
 
 
 @pytest.mark.parametrize(
-    "status,quantity,expected_sent",
+    ("status", "quantity", "expected_sent"),
     [
         (Status.QUEUED, 10, 10),
         (Status.DEFERRED, 10, 10),