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

* ruff as linter and formatter

* Use the ruff formatter

* Address PR feedback + fix lint warnings

* Address PR feedback + fix failing tests

* Select all I rules for ruff linting

* Fix failing tests while keeping linter happy

* Add inline comments for all ignore rules

---------

Co-authored-by: Paulo Machado <[email protected]>

Author: shayancanonical

poetry.lock

@@ -36,22 +36,14 @@ opentelemetry-exporter-otlp-proto-http = "1.21.0"
 optional = true
 
 [tool.poetry.group.format.dependencies]
-black = "^24.0.0"
-isort = "^5.12.0"
+ruff = "^0.4.5"
 
 [tool.poetry.group.lint]
 optional = true
 
 [tool.poetry.group.lint.dependencies]
-black = "^24.0.0"
-isort = "^5.12.0"
-flake8 = "^7.0.0"
-flake8-docstrings = "^1.7.0"
-flake8-copyright = "^0.2.4"
-flake8-builtins = "^2.1.0"
-pyproject-flake8 = "^7.0.0"
-pep8-naming = "^0.13.3"
-codespell = "^2.2.5"
+ruff = "^0.4.5"
+codespell = "^2.3.0"
 shellcheck-py = "^0.9.0.5"
 
 [tool.poetry.group.unit.dependencies]
@@ -92,30 +84,43 @@ log_cli_level = "INFO"
 markers = ["unstable", "juju3", "only_with_juju_secrets", "only_without_juju_secrets"]
 asyncio_mode = "auto"
 
-# Formatting tools configuration
-[tool.black]
+# Linting tools configuration
+[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"
-line_length = 99
+[tool.ruff.lint]
+explicit-preview-rules = true
+select = ["A", "E", "W", "F", "C", "N", "D", "I", "CPY001"]
+ignore = [
+    "D107", # Ignore D107 Missing docstring in __init__
+    "D203", # Ignore D203 1 blank line required before class docstring
+    "D204", # Ignore D204 1 blank line required after class docstring
+    "D213", # Ignore D213 Multi-line docstring summary should start at the second line
+    "D215", # Ignore D215 Section underline is over-indented
+    "D400", # Ignore D400 First line should end with a period
+    "D404", # Ignore D404 First word of the docstring should not be "This"
+    "D406", # Ignore D406 Section name should end with a newline
+    "D407", # Ignore D407 Missing dashed underline after section
+    "D408", # Ignore D408 Section underline should be in the line following section's name
+    "D409", # Ignore D409 Section underline should match the length of its name
+    "D413", # Ignore D413 Missing blank line after last section
+    "E501", # Ignore E501 Line too long
+]
 
-# 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 = ["W503", "E501", "D107"]
-# D100, D101, D102, D103: Ignore missing docstrings in tests
-per-file-ignores = ["tests/*:D100,D101,D102,D103,D104"]
-docstring-convention = "google"
+[tool.ruff.lint.per-file-ignores]
+"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"

pyproject.toml

@@ -201,17 +201,11 @@ def _mysql(self) -> MySQL:
             self.app_peer_data["cluster-set-domain-name"],
             self.get_secret("app", ROOT_PASSWORD_KEY),  # pyright: ignore [reportArgumentType]
             SERVER_CONFIG_USERNAME,
-            self.get_secret(
-                "app", SERVER_CONFIG_PASSWORD_KEY
-            ),  # pyright: ignore [reportArgumentType]
+            self.get_secret("app", SERVER_CONFIG_PASSWORD_KEY),  # pyright: ignore [reportArgumentType]
             CLUSTER_ADMIN_USERNAME,
-            self.get_secret(
-                "app", CLUSTER_ADMIN_PASSWORD_KEY
-            ),  # pyright: ignore [reportArgumentType]
+            self.get_secret("app", CLUSTER_ADMIN_PASSWORD_KEY),  # pyright: ignore [reportArgumentType]
             MONITORING_USERNAME,
-            self.get_secret(
-                "app", MONITORING_PASSWORD_KEY
-            ),  # pyright: ignore [reportArgumentType]
+            self.get_secret("app", MONITORING_PASSWORD_KEY),  # pyright: ignore [reportArgumentType]
             BACKUPS_USERNAME,
             self.get_secret("app", BACKUPS_PASSWORD_KEY),  # pyright: ignore [reportArgumentType]
             self.unit.get_container(CONTAINER_NAME),

src/charm.py

@@ -3,6 +3,7 @@
 # See LICENSE file for licensing details.
 
 """Structured configuration for the MySQL charm."""
+
 import configparser
 import logging
 import re

src/config.py

@@ -112,10 +112,6 @@ class MySQLDeleteTempBackupDirectoryError(Error):
     """Exception raised when there is an error deleting the temp backup directory."""
 
 
-class MySQLDeleteTempRestoreDirectory(Error):
-    """Exception raised when there is an error deleting the temp restore directory."""
-
-
 class MySQL(MySQLBase):
     """Class to encapsulate all operations related to the MySQL instance and cluster.
 
@@ -190,9 +186,12 @@ def fix_data_dir(self, container: Container) -> None:
         if paths[0].user != MYSQL_SYSTEM_USER or paths[0].group != MYSQL_SYSTEM_GROUP:
             logger.debug(f"Changing ownership to {MYSQL_SYSTEM_USER}:{MYSQL_SYSTEM_GROUP}")
             try:
-                container.exec(
-                    ["chown", "-R", f"{MYSQL_SYSTEM_USER}:{MYSQL_SYSTEM_GROUP}", MYSQL_DATA_DIR]
-                )
+                container.exec([
+                    "chown",
+                    "-R",
+                    f"{MYSQL_SYSTEM_USER}:{MYSQL_SYSTEM_GROUP}",
+                    MYSQL_DATA_DIR,
+                ])
             except ExecError as e:
                 logger.error(f"Exited with code {e.exit_code}. Stderr:\n{e.stderr}")
                 raise MySQLInitialiseMySQLDError(e.stderr or "")
@@ -586,6 +585,7 @@ def _run_mysqlsh_script(
         Args:
             script: mysql-shell python script string
             verbose: mysqlsh verbosity level
+            timeout: timeout to wait for the script
 
         Returns:
             stdout of the script

src/mysql_k8s_helpers.py

@@ -156,14 +156,12 @@ def log_rollback_instructions(self) -> None:
             run_action = "run-action"
             wait = " --wait"
         logger.critical(
-            "\n".join(
-                (
-                    "Upgrade failed, follow the instructions below to rollback:",
-                    f"  1 - Run `juju {run_action} {self.charm.app.name}/leader pre-upgrade-check{wait}` to configure rollback",
-                    f"  2 - Run `juju refresh --revision <previous-revision> {self.charm.app.name}` to initiate the rollback",
-                    f"  3 - Run `juju {run_action} {self.charm.app.name}/leader resume-upgrade{wait}` to resume the rollback",
-                )
-            )
+            "\n".join((
+                "Upgrade failed, follow the instructions below to rollback:",
+                f"  1 - Run `juju {run_action} {self.charm.app.name}/leader pre-upgrade-check{wait}` to configure rollback",
+                f"  2 - Run `juju refresh --revision <previous-revision> {self.charm.app.name}` to initiate the rollback",
+                f"  3 - Run `juju {run_action} {self.charm.app.name}/leader resume-upgrade{wait}` to resume the rollback",
+            ))
         )
 
     def _pre_upgrade_prepare(self) -> None:

src/upgrade.py

@@ -216,6 +216,7 @@ async def fetch_credentials(unit: Unit, username: str = None) -> Dict:
 
     Args:
         unit: The juju unit on which to run the get-password action for credentials
+        username: The username for which to fetch credentials
 
     Returns:
         A dictionary with the server config username and password
@@ -231,6 +232,8 @@ async def rotate_credentials(unit: Unit, username: str = None, password: str = N
 
     Args:
         unit: The juju unit on which to run the set-password action for credentials
+        username: The username for which to rotate credentials
+        password: The password with which to rotate credentials
 
     Returns:
         A dictionary with the action result
@@ -625,7 +628,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:
@@ -719,7 +722,7 @@ async def dispatch_custom_event_for_logrotate(ops_test: OpsTest, unit_name: str)
     dispatch_command = juju_exec.strip() or juju_run.strip()
     unit_label = unit_name.replace("/", "-")
 
-    return_code, stdout, stderr = await ops_test.juju(
+    return_code, _, _ = await ops_test.juju(
         "ssh",
         unit_name,
         dispatch_command,

tests/integration/helpers.py

@@ -248,12 +248,10 @@ def deploy_chaos_mesh(namespace: str) -> None:
     logger.info("Deploying Chaos Mesh")
 
     subprocess.check_output(
-        " ".join(
-            [
-                "tests/integration/high_availability/scripts/deploy_chaos_mesh.sh",
-                namespace,
-            ]
-        ),
+        " ".join([
+            "tests/integration/high_availability/scripts/deploy_chaos_mesh.sh",
+            namespace,
+        ]),
         shell=True,
         env=env,
     )

tests/integration/high_availability/high_availability_helpers.py

@@ -49,9 +49,7 @@ def first_model(ops_test: OpsTest) -> Optional[Model]:
 
 
 @pytest.fixture(scope="module")
-async def second_model(
-    ops_test: OpsTest, first_model, request
-) -> Model:  # pyright: ignore [reportInvalidTypeForm]
+async def second_model(ops_test: OpsTest, first_model, request) -> Model:  # pyright: ignore [reportInvalidTypeForm]
     """Create and return the second model."""
     second_model_name = f"{first_model.info.name}-other"
     await ops_test._controller.add_model(second_model_name)
@@ -426,7 +424,7 @@ async def get_max_written_value(first_model: Model, second_model: Model) -> list
     await juju_.run_action(application_unit, "stop-continuous-writes")
 
     sleep(5)
-    results = list()
+    results = []
 
     logger.info("Querying max value on all units")
     for unit in first_model_units + second_model_units:

tests/integration/high_availability/test_async_replication.py

@@ -382,7 +382,7 @@ async def test_network_cut_affecting_an_instance(
 @pytest.mark.unstable
 async def test_graceful_full_cluster_crash_test(ops_test: OpsTest, continuous_writes) -> None:
     """Test to send SIGTERM to all units and then ensure that the cluster recovers."""
-    mysql_application_name, application_name = await high_availability_test_setup(ops_test)
+    mysql_application_name, _ = await high_availability_test_setup(ops_test)
 
     logger.info("Ensure there are 3 online mysql members")
     assert await ensure_n_online_mysql_members(