Add formatter configs, run formatter

- Added configs for leniency on doc string and comment length
- Removed the Ruff GH action in favour of our current style of checking in run_tests.sh

.github/workflows/test.yaml

@@ -39,10 +39,6 @@ jobs:
       run: poetry lock --check
     - name: Install requirements
       run: poetry install --with test
-    - name: Lint and format checks
-      uses: chartboost/ruff-action@v1
-      with:
-        src: "."
     - name: Run tests
       run: poetry run make test
     - name: Upload pytest logs on failure

Makefile

@@ -37,9 +37,8 @@ clean:
 
 .PHONY: format
 format:
-	poetry run isort .
-	poetry run black --config pyproject.toml .
-	poetry run flake8 .
+	ruff check --select I --fix .
+	ruff format .
 	poetry run mypy .
 
 .PHONY: smoke-test

app/celery/scheduled_tasks.py

@@ -215,9 +215,9 @@ def replay_created_notifications():
 
         if len(notifications_to_resend) > 0:
             current_app.logger.info(
-                "Sending {} {} notifications "
-                "to the delivery queue because the notification "
-                "status was created.".format(len(notifications_to_resend), notification_type)
+                "Sending {} {} notifications " "to the delivery queue because the notification " "status was created.".format(
+                    len(notifications_to_resend), notification_type
+                )
             )
 
         for n in notifications_to_resend:

app/celery/tasks.py

@@ -594,9 +594,7 @@ def send_inbound_sms_to_service(self, inbound_sms_id, service_id):
             except self.MaxRetriesExceededError:
                 current_app.logger.error(
                     """Retry: send_inbound_sms_to_service has retried the max number of
-                     times for service: {} and  inbound_sms {}""".format(
-                        service_id, inbound_sms_id
-                    )
+                     times for service: {} and  inbound_sms {}""".format(service_id, inbound_sms_id)
                 )
 
 

app/encryption.py

@@ -37,7 +37,7 @@ def init_app(self, app: Any, secret_key: str | List[str], salt: str) -> None:
             salt (str): The salt to use for signing.
         """
         self.app = app
-        self.secret_key = cast(List[str], [secret_key] if type(secret_key) is str else secret_key)
+        self.secret_key = cast(List[str], [secret_key] if isinstance(secret_key, str) else secret_key)
         self.serializer = URLSafeSerializer(secret_key)
         self.salt = salt
 

app/service/callback_rest.py

@@ -148,8 +148,8 @@ def handle_sql_error(e, table_name):
         and hasattr(e.orig, "pgerror")
         and e.orig.pgerror
         and (
-            'insert or update on table "{0}" violates '
-            'foreign key constraint "{0}_service_id_fkey"'.format(table_name) in e.orig.pgerror
+            'insert or update on table "{0}" violates ' 'foreign key constraint "{0}_service_id_fkey"'.format(table_name)
+            in e.orig.pgerror
         )
     ):
         return jsonify(result="error", message="No result found"), 404

pyproject.toml

@@ -3,6 +3,7 @@ target-version = "py310"
 exclude = ["venv*", "__pycache__", "node_modules", "cache", "migrations", "build"]
 extend-include = ['(app|migrations|tests)/.*\.pyi?$']
 src = ["app", "migrations", "tests"]
+# Ruff formatter will wrap lines at a length of 130 characters.
 line-length = 130
 indent-width = 4
 
@@ -17,9 +18,14 @@ select = [
     "I001",
     "I002"
 ]
-
 ignore = ["E203", "E501", "E402"]
 
+# Provide line length leeway for docstrings
+[tool.ruff.lint.pycodestyle]
+max-doc-length = 170
+# Enforce doc string format? (google, numpy or pep257)
+# convention = "google"
+
 [tool.ruff.format]
 # Match black formatting
 # Double quotes for strings.

scripts/enlarge_db/enlarge_db.py

@@ -1,4 +1,3 @@
-
 import argparse
 import sys
 from datetime import datetime
@@ -23,19 +22,37 @@ def create_notifications(n: int, ref: str) -> List[NotificationHistory]:
             template_version=1,
             service_id=Config.NOTIFY_SERVICE_ID,
             notification_type="email",
-            key_type='normal',
+            key_type="normal",
             client_reference=ref,
         )
         for _ in range(n)
     ]
     return notifications
 
 
-if __name__ == '__main__':
+if __name__ == "__main__":
     parser = argparse.ArgumentParser()
-    parser.add_argument("-n", "--notifications", default=1, type=int, help="number of notifications to add to the notification_history table (default 1)")
-    parser.add_argument("-r", "--reference", default="manually created", type=str, help="client reference to use for the notifications (default 'manually created')")
-    parser.add_argument("-c", "--chunksize", default=DEFAULT_CHUNK_SIZE, type=int, help=f"chunk size for bulk_save_objects (default {DEFAULT_CHUNK_SIZE})")
+    parser.add_argument(
+        "-n",
+        "--notifications",
+        default=1,
+        type=int,
+        help="number of notifications to add to the notification_history table (default 1)",
+    )
+    parser.add_argument(
+        "-r",
+        "--reference",
+        default="manually created",
+        type=str,
+        help="client reference to use for the notifications (default 'manually created')",
+    )
+    parser.add_argument(
+        "-c",
+        "--chunksize",
+        default=DEFAULT_CHUNK_SIZE,
+        type=int,
+        help=f"chunk size for bulk_save_objects (default {DEFAULT_CHUNK_SIZE})",
+    )
     args = parser.parse_args()
 
     app = Flask("enlarge_db")

scripts/load_test/load_test.py

@@ -41,15 +41,15 @@ def send_bulk_email(self, template: str, count: int):
         json = {
             "name": f"bulk emails {datetime.utcnow().isoformat()}",
             "template_id": template,
-            "csv": rows_to_csv([["email address"], *job_lines(self.email_address, count)])
+            "csv": rows_to_csv([["email address"], *job_lines(self.email_address, count)]),
         }
         self.client.post("/v2/notifications/bulk", json=json, headers=self.headers, timeout=60)
 
     def send_bulk_sms(self, template: str, count: int):
         json = {
             "name": f"bulk sms {datetime.utcnow().isoformat()}",
             "template_id": template,
-            "csv": rows_to_csv([["phone_number"], *job_lines(self.phone_number, count)])
+            "csv": rows_to_csv([["phone_number"], *job_lines(self.phone_number, count)]),
         }
         self.client.post("/v2/notifications/bulk", json=json, headers=self.headers, timeout=60)
 

scripts/resign_database.py

@@ -17,7 +17,7 @@
 from dotenv import load_dotenv
 from flask import Flask
 
-sys.path.append('..')  # needed so we can find app (as run from scripts/ folder)
+sys.path.append("..")  # needed so we can find app (as run from scripts/ folder)
 
 from flask import current_app  # noqa: E402
 
@@ -40,10 +40,12 @@ def resign_all(chunk: int, resign: bool, unsafe: bool, notifications: bool):
 
 if __name__ == "__main__":
     parser = argparse.ArgumentParser()
-    parser.add_argument("-n", "--notifications", default=False, action='store_true', help="resign notifications (default false)")
-    parser.add_argument("-c", "--chunk", default=25000, type=int, help="size of chunks of notifications to resign at a time (default 25000)")
-    parser.add_argument("-r", "--resign", default=False, action='store_true', help="resign columns (default false)")
-    parser.add_argument("-u", "--unsafe", default=False, action='store_true', help="ignore bad signatures (default false)")
+    parser.add_argument("-n", "--notifications", default=False, action="store_true", help="resign notifications (default false)")
+    parser.add_argument(
+        "-c", "--chunk", default=25000, type=int, help="size of chunks of notifications to resign at a time (default 25000)"
+    )
+    parser.add_argument("-r", "--resign", default=False, action="store_true", help="resign columns (default false)")
+    parser.add_argument("-u", "--unsafe", default=False, action="store_true", help="ignore bad signatures (default false)")
 
     args = parser.parse_args()
 

scripts/run_tests.sh

@@ -23,6 +23,12 @@ function display_result {
 make test-requirements
 display_result $? 1 "Requirements check"
 
+ruff check .
+display_result $? 1 "Code style check"
+
+ruff check --select I .
+display_result $? 1 "Import order check"
+
 mypy .
 display_result $? 1 "Type check"
 

scripts/soak_test/soak_test_all_servers.py

@@ -12,9 +12,7 @@ class MultipleHostsUser(HttpUser):
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
 
-        self.admin_client = HttpSession(
-            base_url=self.host, request_event=self.client.request_event, user=self
-        )
+        self.admin_client = HttpSession(base_url=self.host, request_event=self.client.request_event, user=self)
 
         self.api_client = HttpSession(
             base_url=url_with_prefix(self.host, "api"), request_event=self.client.request_event, user=self

scripts/soak_test/soak_test_send_notification.py

@@ -10,7 +10,7 @@
 @events.init_command_line_parser.add_listener
 def _(parser):
     parser.add_argument("--ref", type=str, default="test", help="reference")
-    parser.add_argument("--sms", action='store_true', help="send sms")
+    parser.add_argument("--sms", action="store_true", help="send sms")
 
 
 class NotifyApiUser(HttpUser):

tests-perf/locust/locust-notifications-with-fail.py

@@ -11,15 +11,17 @@
 from locust import HttpUser, constant_pacing, task
 
 load_dotenv()
-NotifyApiUserTemplateGroup = make_dataclass('NotifyApiUserTemplateGroup', [
-    'email_id',
-    'email_with_attachment_id',
-    'email_with_link_id',
-])
+NotifyApiUserTemplateGroup = make_dataclass(
+    "NotifyApiUserTemplateGroup",
+    [
+        "email_id",
+        "email_with_attachment_id",
+        "email_with_link_id",
+    ],
+)
 
 
 class NotifyApiUser(HttpUser):
-
     wait_time = constant_pacing(60)
     host = os.getenv("PERF_TEST_DOMAIN", "https://api.staging.notification.cdssandbox.xyz")
 
@@ -69,10 +71,10 @@ def send_email_with_link_notifications(self):
 
     def __email_json(self, template_id, personalisation={}):
         email_invalid = [
-            "[email protected]", 
-            "[email protected]", 
-            "[email protected]", 
-            "[email protected]"
+            "[email protected]",
+            "[email protected]",
+            "[email protected]",
+            "[email protected]",
         ]
         email_index = random.randint(0, len(email_invalid) - 1)
         email = email_invalid[email_index] if random.random() <= self.fail_rate else self.email_success

tests-perf/locust/locust-notifications.py

@@ -15,17 +15,19 @@
 from tests_smoke.smoke.common import job_line, rows_to_csv  # type: ignore
 
 load_dotenv()
-NotifyApiUserTemplateGroup = make_dataclass('NotifyApiUserTemplateGroup', [
-    'bulk_email_id',
-    'email_id',
-    'email_with_attachment_id',
-    'email_with_link_id',
-    'sms_id',
-])
+NotifyApiUserTemplateGroup = make_dataclass(
+    "NotifyApiUserTemplateGroup",
+    [
+        "bulk_email_id",
+        "email_id",
+        "email_with_attachment_id",
+        "email_with_link_id",
+        "sms_id",
+    ],
+)
 
 
 class NotifyApiUser(HttpUser):
-
     wait_time = constant_pacing(60)
     host = os.getenv("PERF_TEST_DOMAIN", "https://api.staging.notification.cdssandbox.xyz")
 
@@ -80,17 +82,14 @@ def send_bulk_notifications(self):
         json = {
             "name": f"My bulk name {datetime.utcnow().isoformat()}",
             "template_id": self.template_group.bulk_email_id,
-            "csv": rows_to_csv([["email address", "application_file"], *job_line(self.email, 2)])
+            "csv": rows_to_csv([["email address", "application_file"], *job_line(self.email, 2)]),
         }
 
         self.client.post("/v2/notifications/bulk", json=json, headers=self.headers)
 
     @task(16)
     def send_sms_notifications(self):
-        json = {
-            "phone_number": self.phone_number,
-            "template_id": self.template_group.sms_id
-        }
+        json = {"phone_number": self.phone_number, "template_id": self.template_group.sms_id}
 
         self.client.post("/v2/notifications/sms", json=json, headers=self.headers)
 

tests-perf/waf-rules/locust-trigger-rate-limit-admin.py

@@ -17,7 +17,6 @@
 
 
 class NotifyAdminUser(HttpUser):
-
     host = "https://notification.canada.ca"
     spawn_rate = 10
     wait_time = constant_pacing(1)

tests-perf/waf-rules/locust-trigger-rate-limit-documentation.py

@@ -14,7 +14,6 @@
 
 
 class NotifyDocumentationUser(HttpUser):
-
     host = "https://documentation.notification.canada.ca/"
     spawn_rate = 10
     wait_time = constant_pacing(1)

tests/app/celery/test_nightly_tasks.py

@@ -257,9 +257,7 @@ def test_timeout_notifications_sends_status_update_to_service(client, sample_tem
 
 
 def test_send_daily_performance_stats_calls_does_not_send_if_inactive(client, mocker):
-    send_mock = mocker.patch(
-        "app.celery.nightly_tasks.total_sent_notifications.send_total_notifications_sent_for_day_stats"
-    )  # noqa
+    send_mock = mocker.patch("app.celery.nightly_tasks.total_sent_notifications.send_total_notifications_sent_for_day_stats")  # noqa
 
     with patch.object(PerformancePlatformClient, "active", new_callable=PropertyMock) as mock_active:
         mock_active.return_value = False
@@ -272,9 +270,7 @@ def test_send_daily_performance_stats_calls_does_not_send_if_inactive(client, mo
 def test_send_total_sent_notifications_to_performance_platform_calls_with_correct_totals(
     notify_db_session, sample_template, sample_email_template, mocker
 ):
-    perf_mock = mocker.patch(
-        "app.celery.nightly_tasks.total_sent_notifications.send_total_notifications_sent_for_day_stats"
-    )  # noqa
+    perf_mock = mocker.patch("app.celery.nightly_tasks.total_sent_notifications.send_total_notifications_sent_for_day_stats")  # noqa
 
     today = date(2016, 6, 11)
     create_ft_notification_status(utc_date=today, template=sample_template)

tests/app/celery/test_scheduled_tasks.py

@@ -457,8 +457,9 @@ def test_check_templated_letter_state_during_bst(mocker, sample_letter_template)
     check_templated_letter_state()
 
     message = (
-        "2 letters were created before 17.30 yesterday and still have 'created' status. "
-        "Notifications: ['{}', '{}']".format(noti_1.id, noti_2.id)
+        "2 letters were created before 17.30 yesterday and still have 'created' status. " "Notifications: ['{}', '{}']".format(
+            noti_1.id, noti_2.id
+        )
     )
 
     mock_logger.assert_called_once_with(message)
@@ -491,8 +492,9 @@ def test_check_templated_letter_state_during_utc(mocker, sample_letter_template)
     check_templated_letter_state()
 
     message = (
-        "2 letters were created before 17.30 yesterday and still have 'created' status. "
-        "Notifications: ['{}', '{}']".format(noti_1.id, noti_2.id)
+        "2 letters were created before 17.30 yesterday and still have 'created' status. " "Notifications: ['{}', '{}']".format(
+            noti_1.id, noti_2.id
+        )
     )
 
     mock_logger.assert_called_once_with(message)