Apply more Ruff linter rules

.cookiecutter/includes/ruff/lint/per_file_ignores/tests/tail

@@ -0,0 +1,6 @@
+"PT006", # Enforces a consistent style for the type of the `argnames` parameter to
+         # pytest.mark.parametrize. We have too many pre-existing violations of
+         # this.
+"PT007", # Enforces a consistent style for the type of the `argvalues` parameter to
+         # pytest.mark.parametrize. We have too many pre-existing violations of
+         # this.

bin/run_data_task.py

@@ -4,7 +4,7 @@
 This is a general mechanism for running tasks defined in SQL, however it's
 currently only used to perform the aggregations and mappings required for
 reporting.
-"""
+"""  # noqa: EXE002
 
 from argparse import ArgumentParser
 
@@ -47,7 +47,7 @@
 def main():
     args = parser.parse_args()
 
-    with bootstrap(args.config_file) as env:  # noqa: PLR1702
+    with bootstrap(args.config_file) as env:
         request = env["request"]
         settings = env["registry"].settings
 
@@ -62,18 +62,18 @@ def main():
         )
 
         # Run the update in a transaction, so we roll back if it goes wrong
-        with request.db.bind.connect() as connection:
+        with request.db.bind.connect() as connection:  # noqa: SIM117
             with connection.begin():
                 for script in scripts:
                     if args.no_python and isinstance(script, PythonScript):
-                        print(f"Skipping: {script}")
+                        print(f"Skipping: {script}")  # noqa: T201
                         continue
 
                     for step in script.execute(connection, dry_run=args.dry_run):
                         if args.dry_run:
-                            print("Dry run!")
+                            print("Dry run!")  # noqa: T201
 
-                        print(step.dump(indent="    ") + "\n")
+                        print(step.dump(indent="    ") + "\n")  # noqa: T201
 
 
 if __name__ == "__main__":

lms/config.py

@@ -31,15 +31,15 @@ def _aes_to_16_chars(string):
     try:
         return string.encode("ascii")[0:16] if string else None
     except UnicodeEncodeError as err:
-        raise SettingError("LMS_SECRET must contain only ASCII characters") from err
+        raise SettingError("LMS_SECRET must contain only ASCII characters") from err  # noqa: EM101, TRY003
 
 
 @dataclass(frozen=True)
 class _Setting:
     """The properties of a setting and how to read it."""
 
     name: str
-    read_from: str = None  # type: ignore
+    read_from: str = None  # type: ignore  # noqa: PGH003
     value_mapper: Callable | None = None
 
     def __post_init__(self):

lms/db/_columns.py

@@ -2,13 +2,13 @@
 from sqlalchemy.orm import Mapped, mapped_column
 
 
-def varchar_enum(  # noqa: PLR0913, PLR0917
+def varchar_enum(  # noqa: PLR0913
     enum,
     default=None,
     max_length=64,
-    nullable=False,
+    nullable=False,  # noqa: FBT002
     server_default=None,
-    unique=False,
+    unique=False,  # noqa: FBT002
 ) -> Mapped:
     """Return a SA column type to store the python enum.Enum as a varchar in a table."""
     return mapped_column(

lms/db/_util.py

@@ -3,13 +3,13 @@
 from sqlalchemy.orm import Query
 
 
-def compile_query(query: Query | Select, literal_binds: bool = True) -> str:
+def compile_query(query: Query | Select, literal_binds: bool = True) -> str:  # noqa: FBT001, FBT002
     """
     Return the SQL representation of `query` for postgres.
 
     :param literal_binds: Whether or not replace the query parameters by their values.
     """
-    if isinstance(query, Query):
+    if isinstance(query, Query):  # noqa: SIM108
         # Support for SQLAlchemy 1.X style queryies, eg: db.query(Model).filter_by()
         statement = query.statement
     else:

lms/events/event.py

@@ -1,5 +1,5 @@
+from collections.abc import Mapping
 from dataclasses import asdict, dataclass, field, fields
-from typing import Mapping
 
 from pyramid.request import Request
 from sqlalchemy import inspect
@@ -109,7 +109,7 @@ def from_instance(cls, instance, **kwargs):
         changes = {}
         instance_details = inspect(instance)
         for attr in instance_details.attrs:
-            history = instance_details.get_history(attr.key, True)
+            history = instance_details.get_history(attr.key, True)  # noqa: FBT003
 
             if not history.has_changes():
                 continue

lms/events/subscribers.py

@@ -7,5 +7,5 @@
 @subscriber(BaseEvent)
 def handle_event(event: BaseEvent):
     """Record the event in the Event model's table."""
-    assert event.request
+    assert event.request  # noqa: S101
     event.request.find_service(EventService).insert_event(event)

lms/extensions/feature_flags/__init__.py

@@ -176,7 +176,7 @@ def feature(request, feature_flag_name):
 
     def add_feature_flag_providers(_config, *providers):
         """Adapt feature_flags.add_providers()."""
-        providers = [config.maybe_dotted(provider) for provider in providers]  # type:ignore
+        providers = [config.maybe_dotted(provider) for provider in providers]  # type:ignore  # noqa: PGH003
         return feature_flags.add_providers(*providers)
 
     # Register the Pyramid request method and config directive. These are this

lms/extensions/feature_flags/_helpers.py

@@ -63,8 +63,8 @@ def __init__(self, name, request):
         try:
             self._secret = request.registry.settings["feature_flags_cookie_secret"]
         except KeyError as err:
-            raise SettingError(
-                "The feature_flags_cookie_secret deployment setting is required"
+            raise SettingError(  # noqa: TRY003
+                "The feature_flags_cookie_secret deployment setting is required"  # noqa: EM101
             ) from err
 
         self._name = name

lms/extensions/feature_flags/_providers.py

@@ -11,8 +11,8 @@
 
 __all__ = [
     "config_file_provider",
-    "envvar_provider",
     "cookie_provider",
+    "envvar_provider",
     "query_string_provider",
 ]
 

lms/extensions/feature_flags/views/cookie_form.py

@@ -23,7 +23,7 @@ def get(self):
         return {
             "flags": flags,
             # The final state of each feature flag
-            "state": {flag: self._request.feature(flag) for flag in flags.keys()},
+            "state": {flag: self._request.feature(flag) for flag in flags.keys()},  # noqa: SIM118
         }
 
     @view_config(request_method="POST")

lms/migrations/versions/088813d3e6f8_email_digest_enable_02.py

@@ -24,4 +24,3 @@ def upgrade():
 
 def downgrade():
     """No downgrade section as we might manually change some values after running `upgrade`."""
-    pass

lms/migrations/versions/0c52a13c6cad_canvas_sso_urls.py

@@ -49,9 +49,9 @@ def upgrade() -> None:
             UPDATE lti_registration
             SET {field} = '{new_url}'
             WHERE {field} ='{old_url}'
-            """
+            """  # noqa: S608
         )
-        print(f"\tUpdated lti_registration.{field}:", result.rowcount)
+        print(f"\tUpdated lti_registration.{field}:", result.rowcount)  # noqa: T201
 
 
 def downgrade() -> None:
@@ -63,6 +63,6 @@ def downgrade() -> None:
             UPDATE lti_registration
             SET {field} = '{old_url}'
             WHERE {field} ='{new_url}'
-            """
+            """  # noqa: S608
         )
-        print(f"\tDowngraded lti_registration.{field}:", result.rowcount)
+        print(f"\tDowngraded lti_registration.{field}:", result.rowcount)  # noqa: T201

lms/migrations/versions/396f46318023_events_backfill.py

@@ -62,7 +62,7 @@ def upgrade():
         ORDER BY lti_launches.created ASC
     """
     )
-    print("\tInserted lti_launches rows into events:", result.rowcount)
+    print("\tInserted lti_launches rows into events:", result.rowcount)  # noqa: T201
 
 
 def downgrade():
@@ -77,5 +77,5 @@ def downgrade():
             -- will have empty assignments.
             AND assignment_id is null
             AND timestamp < '{CUT_OFF_DATE}'
-    """
+    """  # noqa: S608
     )

lms/migrations/versions/3a786e91f59c_email_digest_enable_04.py

@@ -34,9 +34,8 @@ def upgrade():
             application_instances.id = candidates.id"""
         )
     )
-    print("\tEnabled email digest in new AIs", result.rowcount)
+    print("\tEnabled email digest in new AIs", result.rowcount)  # noqa: T201
 
 
 def downgrade():
     """No downgrade section as we might manually change some values after running `upgrade`."""
-    pass

lms/migrations/versions/52755322151e_db_migration_to_back_fill_organizations.py

@@ -103,7 +103,7 @@ def connected_subgraphs(edges):
     # Put every node in a group on its own
     group_to_nodes = {node: {node} for node in itertools.chain(*edges)}
     # ... and record it's location (which will change)
-    node_to_group = {node: node for node in group_to_nodes.keys()}
+    node_to_group = {node: node for node in group_to_nodes.keys()}  # noqa: SIM118
 
     # Repeatedly merge groups if they are joined by an edge
     for left_node, right_node in edges:
@@ -150,20 +150,20 @@ def pick_name(names):
 def upgrade():
     db_session = sa.orm.Session(bind=op.get_bind())
 
-    print("Clearing old organizations...")
+    print("Clearing old organizations...")  # noqa: T201
     op.execute("UPDATE application_instances SET organization_id = NULL")
     op.execute("DELETE FROM organization")
 
-    with open(__file__.replace(".py", ".sql"), encoding="utf-8") as handle:
+    with open(__file__.replace(".py", ".sql"), encoding="utf-8") as handle:  # noqa: PTH123
         query = handle.read()
 
-    print("Detecting GUIDs...")
+    print("Detecting GUIDs...")  # noqa: T201
     edges = list(db_session.execute(query))
 
-    print("Grouping GUIDs...")
+    print("Grouping GUIDs...")  # noqa: T201
     grouped_guids = connected_subgraphs(edges)
 
-    print("Creating new organizations...")
+    print("Creating new organizations...")  # noqa: T201
     total = 0
 
     for ai_ids in grouped_guids:
@@ -179,15 +179,15 @@ def upgrade():
             name=pick_name([ai.tool_consumer_instance_name for ai in ais])
         )
         total += 1
-        print(
+        print(  # noqa: T201
             f"\tCreating '{organization.name}' for {len(ais)} application instance(s)"
         )
         for ai in ais:
             ai.organization = organization
 
         db_session.add(organization)
 
-    print(f"Created {total} organization(s). Done")
+    print(f"Created {total} organization(s). Done")  # noqa: T201
 
 
 def downgrade():

lms/migrations/versions/52ff45973d5b_remove_ext_id.py

@@ -29,7 +29,7 @@ def upgrade():
             AND resource_link_id IS NULL"""
     )
     # We expect this to affect no more that 3 rows after checking the data in production.
-    assert result.rowcount <= 3, "Trying to delete more rows that expected"
+    assert result.rowcount <= 3, "Trying to delete more rows that expected"  # noqa: S101
 
     # Make resource_link_id not nullable
     op.alter_column(

lms/migrations/versions/9207d2192903_email_digest_enable_03.py

@@ -24,4 +24,4 @@ def upgrade():
 
 def downgrade():
     """No downgrade section as we might manually change some values after running `upgrade`."""
-    pass
+    pass  # noqa: PIE790

lms/migrations/versions/a9e5a33e96b7_backfill_lmsuserassignmentmembership.py

@@ -43,7 +43,7 @@ def upgrade() -> None:
         )
     )
 
-    pass
+    pass  # noqa: PIE790
 
 
 def downgrade() -> None:

lms/migrations/versions/b11713314988_email_digest_enable_01.py

@@ -48,7 +48,7 @@ def enable_email_digest(conn, limit=500):
         ),
         limit=limit,
     )
-    print("\tEnabled email digest in new AIs", result.rowcount)
+    print("\tEnabled email digest in new AIs", result.rowcount)  # noqa: T201
 
 
 def upgrade():
@@ -58,4 +58,4 @@ def upgrade():
 
 def downgrade():
     """No downgrade section as we might manually change some values after running `upgrade`."""
-    pass
+    pass  # noqa: PIE790

lms/migrations/versions/b512a5cf64ed_public_id_migration.py

@@ -18,7 +18,7 @@ def upgrade() -> None:
     conn = op.get_bind()
     conn.execute(
         sa.text(
-            f"""UPDATE "organization" set public_id = '{region}.lms.org.' || "public_id";"""
+            f"""UPDATE "organization" set public_id = '{region}.lms.org.' || "public_id";"""  # noqa: S608
         )
     )
 

lms/migrations/versions/db3779df3f44_back_fill_course_and_course_groups_exported_from_h_tables.py

@@ -44,7 +44,7 @@ def upgrade():
             continue
 
         authority_provided_id = group["authority_provided_id"]
-        created = datetime.datetime.strptime(group["created"], "%Y-%m-%dT%H:%M:%S.%fZ")
+        created = datetime.datetime.strptime(group["created"], "%Y-%m-%dT%H:%M:%S.%fZ")  # noqa: DTZ007
 
         # We can't tell which course groups belong to which application instance
         # in the period since we released sections and now, so instead we will
@@ -76,14 +76,14 @@ def upgrade():
     session.commit()
 
     log.info(
-        f"Inserted {rows_inserted_into_course_table} rows into course table with"
+        f"Inserted {rows_inserted_into_course_table} rows into course table with"  # noqa: G004
         " sections enabled"
     )
     log.info(
-        f"Inserted {rows_inserted_into_course_groups_exported_from_h_table} rows into"
+        f"Inserted {rows_inserted_into_course_groups_exported_from_h_table} rows into"  # noqa: G004
         " course_groups_exported_from_h table"
     )
-    log.info(f"Inserted {rows_inserted} rows in total")
+    log.info(f"Inserted {rows_inserted} rows in total")  # noqa: G004
 
 
 def downgrade():
@@ -114,7 +114,7 @@ def is_course_group(session, group):
 def maybe_commit(session, rows_inserted):
     """Commit the session every 1000 rows."""
     if rows_inserted % 1000 == 0:
-        log.info(f"Commit {rows_inserted}")
+        log.info(f"Commit {rows_inserted}")  # noqa: G004
         session.commit()
 
 

lms/migrations/versions/e5a9845d55d7_clean_grouping_and_assignment_titles.py

@@ -22,7 +22,7 @@ def upgrade() -> None:
     """
         )
     )
-    print(f"Assignment titles updated: {result.rowcount}")
+    print(f"Assignment titles updated: {result.rowcount}")  # noqa: T201
 
     result = conn.execute(
         sa.text(
@@ -32,7 +32,7 @@ def upgrade() -> None:
     """
         )
     )
-    print(f"Empty assignment titles: {result.rowcount}")
+    print(f"Empty assignment titles: {result.rowcount}")  # noqa: T201
 
     result = conn.execute(
         sa.text(
@@ -42,7 +42,7 @@ def upgrade() -> None:
     """
         )
     )
-    print(f"Grouping titles updated: {result.rowcount}")
+    print(f"Grouping titles updated: {result.rowcount}")  # noqa: T201
 
 
 def downgrade() -> None:

lms/migrations/versions/f3d631c110bf_canvas_files_enabled.py

@@ -53,9 +53,9 @@ def upgrade():
             )
         """
     )
-    print("\tApplication instances marked with canvas->files_enabled:", result.rowcount)
+    print("\tApplication instances marked with canvas->files_enabled:", result.rowcount)  # noqa: T201
 
 
 def downgrade():
     """No downgrade section as we might manually change some values after running `upgrade`."""
-    pass
+    pass  # noqa: PIE790

lms/models/_hashed_id.py

@@ -11,7 +11,7 @@ def hashed_id(*parts):
     :param *parts: An iterable of objects which can be converted to strings
     :return: A string which can be used as an id
     """
-    hash_object = hashlib.sha1()
+    hash_object = hashlib.sha1()  # noqa: S324
     for part in parts:
         hash_object.update(str(part).encode())