[DPE-3881] Use ruff as a linter and formatter (#292)

## Issue We would like to use ruff as a linter and formatter ## Solution Use ruff as a linter and formatter. Run format on the repo and push up changes as a result of the format
canonical · Jul 25, 2024 · ae2e8eb · ae2e8eb
1 parent 266066c
commit ae2e8eb
Show file tree

Hide file tree

Showing 16 changed files with 161 additions and 414 deletions.
diff --git a/poetry.lock b/poetry.lock
diff --git a/pyproject.toml b/pyproject.toml
@@ -35,21 +35,13 @@ jsonschema = "*"
 optional = true
 
 [tool.poetry.group.format.dependencies]
-black = "^24.4.2"
-isort = "^5.13.2"
+ruff = "^0.4.5"
 
 [tool.poetry.group.lint]
 optional = true
 
 [tool.poetry.group.lint.dependencies]
-black = "^24.4.2"
-isort = "^5.13.2"
-flake8 = "^7.0.0"
-flake8-docstrings = "^1.7.0"
-flake8-copyright = "^0.2.4"
-flake8-builtins = "^2.5.0"
-pyproject-flake8 = "^7.0.0"
-pep8-naming = "^0.14.1"
+ruff = "^0.4.5"
 codespell = "^2.3.0"
 
 [tool.poetry.group.unit.dependencies]
@@ -85,35 +77,40 @@ log_cli_level = "INFO"
 markers = ["unstable"]
 
 # Formatting tools configuration
-[tool.black]
+[tool.ruff]
+# preview and explicit preview are enabled for CPY001
+preview = true
+target-version = "py38"
+src = ["src", "."]
 line-length = 99
-target-version = ["py38"]
 
-[tool.isort]
-profile = "black"
-known_third_party = "mysql.connector"
-
-# Linting tools configuration
-[tool.flake8]
-max-line-length = 99
-max-doc-length = 99
-max-complexity = 10
-exclude = [".git", "__pycache__", ".tox", "build", "dist", "*.egg_info", "venv"]
-select = ["E", "W", "F", "C", "N", "R", "D", "H"]
-# Ignore W503, E501 because using black creates errors with this
-# Ignore D107 Missing docstring in __init__
-# Ignore D105 Missing docstring in magic method
-# Ignore D415 Docstring first line punctuation (doesn't make sense for properties)
-# Ignore D403 First word of the first line should be properly capitalized (false positive on "MySQL")
-# Ignore N818 Exception should be named with an Error suffix
-# Ignore D102 Missing docstring in public method (pydocstyle doesn't look for docstrings in super class
-# Ignore W505 So that strings in comments aren't split across lines
-# https://github.com/PyCQA/pydocstyle/issues/309) TODO: add pylint check? https://github.com/PyCQA/pydocstyle/issues/309#issuecomment-1284142716
-ignore = ["W503", "E501", "D107", "D105", "D415", "D403", "N818", "D102", "W505"]
+[tool.ruff.lint]
+explicit-preview-rules = true
+select = ["A", "E", "W", "F", "C", "N", "D", "I", "CPY001"]
+ignore = [
+    # Missing docstring in public method (pydocstyle doesn't look for docstrings in super class
+    # https://github.com/PyCQA/pydocstyle/issues/309) TODO: add pylint check? https://github.com/PyCQA/pydocstyle/issues/309#issuecomment-1284142716
+    "D102",
+    "D105", # Missing docstring in magic method
+    "D107", # Missing docstring in __init__
+    "D403", # First word of the first line should be capitalized (false positive on "MySQL")
+    "D415", # Docstring first line punctuation (doesn't make sense for properties)
+    "E501", # Line too long (because using black creates errors with this)
+    "N818", # Exception name should be named with an Error suffix
+    "W505", # Doc line too long (so that strings in comments aren't split across lines)
+]
+
+[tool.ruff.lint.per-file-ignores]
 # D100, D101, D102, D103: Ignore missing docstrings in tests
-per-file-ignores = ["tests/*:D100,D101,D102,D103,D104"]
-docstring-convention = "google"
+"tests/*" = ["D1"]
+
+[tool.ruff.lint.flake8-copyright]
 # Check for properly formatted copyright header in each file
-copyright-check = "True"
-copyright-author = "Canonical Ltd."
-copyright-regexp = "Copyright\\s\\d{4}([-,]\\d{4})*\\s+%(author)s"
+author = "Canonical Ltd."
+notice-rgx = "Copyright\\s\\d{4}([-,]\\d{4})*\\s+"
+
+[tool.ruff.lint.mccabe]
+max-complexity = 10
+
+[tool.ruff.lint.pydocstyle]
+convention = "google"
diff --git a/src/container.py b/src/container.py
@@ -177,7 +177,7 @@ def _run_command(
         command: typing.List[str],
         *,
         timeout: typing.Optional[int],
-        input: str = None,
+        input: str = None,  # noqa: A002 Match subprocess.run()
     ) -> str:
         """Run command in container.
 

diff --git a/src/lifecycle.py b/src/lifecycle.py
@@ -5,6 +5,7 @@
 
 https://juju.is/docs/sdk/a-charms-life
 """
+
 import enum
 import logging
 import typing

diff --git a/src/mysql_shell/__init__.py b/src/mysql_shell/__init__.py
@@ -82,14 +82,12 @@ def render(connection_info: "relations.database_requires.ConnectionInformation")
         error_file = self._container.path("/tmp/mysqlsh_error.json")
         temporary_script_file.write_text(script)
         try:
-            self._container.run_mysql_shell(
-                [
-                    "--no-wizard",
-                    "--python",
-                    "--file",
-                    str(temporary_script_file.relative_to_container),
-                ]
-            )
+            self._container.run_mysql_shell([
+                "--no-wizard",
+                "--python",
+                "--file",
+                str(temporary_script_file.relative_to_container),
+            ])
         except container.CalledProcessError as e:
             logger.exception(
                 f"Failed to run MySQL Shell script:\n{logged_script}\n\nstderr:\n{e.stderr}\n"
@@ -105,8 +103,8 @@ def render(connection_info: "relations.database_requires.ConnectionInformation")
                 raise ShellDBError(**exception)
         except ShellDBError as e:
             if e.code == 2003:
-                logger.exception(server_exceptions.ConnectionError.MESSAGE)
-                raise server_exceptions.ConnectionError
+                logger.exception(server_exceptions.ConnectionError_.MESSAGE)
+                raise server_exceptions.ConnectionError_
             else:
                 logger.exception(
                     f"Failed to run MySQL Shell script:\n{logged_script}\n\nMySQL client error {e.code}\nMySQL Shell traceback:\n{e.traceback_message}\n"
@@ -136,23 +134,22 @@ def create_application_database_and_user(self, *, username: str, database: str)
         attributes = self._get_attributes()
         logger.debug(f"Creating {database=} and {username=} with {attributes=}")
         password = utils.generate_password()
-        self._run_sql(
-            [
-                f"CREATE DATABASE IF NOT EXISTS `{database}`",
-                f"CREATE USER `{username}` IDENTIFIED BY '{password}' ATTRIBUTE '{attributes}'",
-                f"GRANT ALL PRIVILEGES ON `{database}`.* TO `{username}`",
-            ]
-        )
+        self._run_sql([
+            f"CREATE DATABASE IF NOT EXISTS `{database}`",
+            f"CREATE USER `{username}` IDENTIFIED BY '{password}' ATTRIBUTE '{attributes}'",
+            f"GRANT ALL PRIVILEGES ON `{database}`.* TO `{username}`",
+        ])
         logger.debug(f"Created {database=} and {username=} with {attributes=}")
         return password
 
     def add_attributes_to_mysql_router_user(
         self, *, username: str, router_id: str, unit_name: str
     ) -> None:
         """Add attributes to user created during MySQL Router bootstrap."""
-        attributes = self._get_attributes(
-            {"router_id": router_id, "created_by_juju_unit": unit_name}
-        )
+        attributes = self._get_attributes({
+            "router_id": router_id,
+            "created_by_juju_unit": unit_name,
+        })
         logger.debug(f"Adding {attributes=} to {username=}")
         self._run_sql([f"ALTER USER `{username}` ATTRIBUTE '{attributes}'"])
         logger.debug(f"Added {attributes=} to {username=}")

diff --git a/src/relations/cos.py b/src/relations/cos.py
@@ -2,6 +2,7 @@
 # See LICENSE file for licensing details.
 
 """Relation to the cos charms."""
+
 import logging
 import typing
 from dataclasses import dataclass

diff --git a/src/rock.py b/src/rock.py
@@ -116,20 +116,18 @@ def update_mysql_router_service(self, *, enabled: bool, tls: bool = None) -> Non
             startup = ops.pebble.ServiceStartup.ENABLED.value
         else:
             startup = ops.pebble.ServiceStartup.DISABLED.value
-        layer = ops.pebble.Layer(
-            {
-                "services": {
-                    self._SERVICE_NAME: {
-                        "override": "replace",
-                        "summary": "MySQL Router",
-                        "command": command,
-                        "startup": startup,
-                        "user": _UNIX_USERNAME,
-                        "group": _UNIX_USERNAME,
-                    },
+        layer = ops.pebble.Layer({
+            "services": {
+                self._SERVICE_NAME: {
+                    "override": "replace",
+                    "summary": "MySQL Router",
+                    "command": command,
+                    "startup": startup,
+                    "user": _UNIX_USERNAME,
+                    "group": _UNIX_USERNAME,
                 },
-            }
-        )
+            },
+        })
         self._container.add_layer(self._SERVICE_NAME, layer, combine=True)
         # `self._container.replan()` does not stop services that have been disabled
         # Use `restart()` and `stop()` instead
@@ -168,32 +166,28 @@ def update_mysql_router_exporter_service(
                 "MYSQLROUTER_EXPORTER_SERVICE_NAME": self._unit_name.replace("/", "-"),
             }
             if tls:
-                environment.update(
-                    {
-                        "MYSQLROUTER_TLS_CACERT_PATH": certificate_authority_filename,
-                        "MYSQLROUTER_TLS_CERT_PATH": certificate_filename,
-                        "MYSQLROUTER_TLS_KEY_PATH": key_filename,
-                    }
-                )
+                environment.update({
+                    "MYSQLROUTER_TLS_CACERT_PATH": certificate_authority_filename,
+                    "MYSQLROUTER_TLS_CERT_PATH": certificate_filename,
+                    "MYSQLROUTER_TLS_KEY_PATH": key_filename,
+                })
         else:
             startup = ops.pebble.ServiceStartup.DISABLED.value
             environment = {}
 
-        layer = ops.pebble.Layer(
-            {
-                "services": {
-                    self._EXPORTER_SERVICE_NAME: {
-                        "override": "replace",
-                        "summary": "MySQL Router Exporter",
-                        "command": "/start-mysql-router-exporter.sh",
-                        "startup": startup,
-                        "user": _UNIX_USERNAME,
-                        "group": _UNIX_USERNAME,
-                        "environment": environment,
-                    },
+        layer = ops.pebble.Layer({
+            "services": {
+                self._EXPORTER_SERVICE_NAME: {
+                    "override": "replace",
+                    "summary": "MySQL Router Exporter",
+                    "command": "/start-mysql-router-exporter.sh",
+                    "startup": startup,
+                    "user": _UNIX_USERNAME,
+                    "group": _UNIX_USERNAME,
+                    "environment": environment,
                 },
-            }
-        )
+            },
+        })
         self._container.add_layer(self._EXPORTER_SERVICE_NAME, layer, combine=True)
         # `self._container.replan()` does not stop services that have been disabled
         # Use `restart()` and `stop()` instead
@@ -216,20 +210,18 @@ def update_logrotate_executor_service(self, *, enabled: bool) -> None:
             if enabled
             else ops.pebble.ServiceStartup.DISABLED.value
         )
-        layer = ops.pebble.Layer(
-            {
-                "services": {
-                    self._LOGROTATE_EXECUTOR_SERVICE_NAME: {
-                        "override": "replace",
-                        "summary": "Logrotate executor",
-                        "command": "python3 /logrotate_executor.py",
-                        "startup": startup,
-                        "user": _UNIX_USERNAME,
-                        "group": _UNIX_USERNAME,
-                    },
+        layer = ops.pebble.Layer({
+            "services": {
+                self._LOGROTATE_EXECUTOR_SERVICE_NAME: {
+                    "override": "replace",
+                    "summary": "Logrotate executor",
+                    "command": "python3 /logrotate_executor.py",
+                    "startup": startup,
+                    "user": _UNIX_USERNAME,
+                    "group": _UNIX_USERNAME,
                 },
-            }
-        )
+            },
+        })
         self._container.add_layer(self._LOGROTATE_EXECUTOR_SERVICE_NAME, layer, combine=True)
         # `self._container.replan()` does not stop services that have been disabled
         # Use `restart()` and `stop()` instead
@@ -240,7 +232,11 @@ def update_logrotate_executor_service(self, *, enabled: bool) -> None:
 
     # TODO python3.10 min version: Use `list` instead of `typing.List`
     def _run_command(
-        self, command: typing.List[str], *, timeout: typing.Optional[int], input: str = None
+        self,
+        command: typing.List[str],
+        *,
+        timeout: typing.Optional[int],
+        input: str = None,  # noqa: A002 Match subprocess.run()
     ) -> str:
         try:
             process = self._container.exec(

diff --git a/src/server_exceptions.py b/src/server_exceptions.py
@@ -12,7 +12,7 @@ class Error(status_exception.StatusException):
     """MySQL Server unreachable or unhealthy"""
 
 
-class ConnectionError(Error):
+class ConnectionError_(Error):  # noqa: N801 for underscore in name
     """MySQL Server unreachable
 
     MySQL client error 2003

diff --git a/src/workload.py b/src/workload.py
@@ -291,8 +291,8 @@ def _bootstrap_router(self, *, event, tls: bool) -> None:
             elif match := re.fullmatch(r"Error:.*\((?P<code>2[0-9]{3})\)", stderr):
                 code = int(match.group("code"))
                 if code == 2003:
-                    logger.error(server_exceptions.ConnectionError.MESSAGE)
-                    raise server_exceptions.ConnectionError from None
+                    logger.error(server_exceptions.ConnectionError_.MESSAGE)
+                    raise server_exceptions.ConnectionError_ from None
                 else:
                     logger.error(f"Bootstrap failed with MySQL client error {code}")
             raise Exception("Failed to bootstrap router") from None

diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py
@@ -157,7 +157,7 @@ async def delete_file_or_directory_in_unit(
     if path.strip() in ["/", "."]:
         return
 
-    return_code, _, _ = await ops_test.juju(
+    await ops_test.juju(
         "ssh",
         "--container",
         container_name,
@@ -274,7 +274,7 @@ async def ls_la_in_unit(
     Args:
         ops_test: The ops test framework
         unit_name: The name of unit in which to run ls -la
-        path: The path from which to run ls -la
+        directory: The directory from which to run ls -la
         container_name: The container where to run ls -la
 
     Returns:
@@ -592,7 +592,7 @@ async def ensure_all_units_continuous_writes_incrementing(
                             select_all_continuous_writes_sql,
                         )
                     )
-                    numbers = {n for n in range(1, max_written_value)}
+                    numbers = set(range(1, max_written_value))
                     assert (
                         numbers <= all_written_values
                     ), f"Missing numbers in database for unit {unit.name}"

diff --git a/tests/integration/test_log_rotation.py b/tests/integration/test_log_rotation.py
@@ -140,9 +140,10 @@ async def test_log_rotation(ops_test: OpsTest):
         len(ls_la_output) == 2
     ), f"❌ unexpected files/directories in log directory: {ls_la_output}"
     directories = [line.split()[-1] for line in ls_la_output]
-    assert sorted(directories) == sorted(
-        ["mysqlrouter.log", "archive_mysqlrouter"]
-    ), f"❌ unexpected files/directories in log directory: {ls_la_output}"
+    assert sorted(directories) == sorted([
+        "mysqlrouter.log",
+        "archive_mysqlrouter",
+    ]), f"❌ unexpected files/directories in log directory: {ls_la_output}"
 
     logger.info("Ensuring log files was rotated")
     file_contents = await read_contents_from_file_in_unit(