Skip to content

Commit

Permalink
[DPE-3881] Use ruff as a linter and formatter (#162)
Browse files Browse the repository at this point in the history
## Issue
We should use ruff as a linter and formatter

## Solution
Use ruff. Fix lint and format warnings
  • Loading branch information
shayancanonical authored Sep 30, 2024
1 parent 2f14032 commit 2eefed0
Show file tree
Hide file tree
Showing 18 changed files with 150 additions and 413 deletions.
287 changes: 22 additions & 265 deletions poetry.lock

Large diffs are not rendered by default.

72 changes: 35 additions & 37 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,21 +35,13 @@ opentelemetry-exporter-otlp-proto-http = "1.21.0"
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]
Expand Down Expand Up @@ -85,34 +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"

# 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"
2 changes: 1 addition & 1 deletion src/container.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions src/lifecycle.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
https://juju.is/docs/sdk/a-charms-life
"""

import enum
import logging
import typing
Expand Down
1 change: 1 addition & 0 deletions src/machine_upgrade.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
Derived from specification: DA058 - In-Place Upgrades - Kubernetes v2
(https://docs.google.com/document/d/1tLjknwHudjcHs42nzPVBNkHs98XxAOT2BXGGpP7NyEU/)
"""

import json
import logging
import time
Expand Down
28 changes: 12 additions & 16 deletions src/machine_workload.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,23 +26,19 @@ def _get_bootstrap_command(
) -> typing.List[str]:
command = super()._get_bootstrap_command(event=event, connection_info=connection_info)
if self._charm.is_externally_accessible(event=event):
command.extend(
[
"--conf-bind-address",
"0.0.0.0",
]
)
command.extend([
"--conf-bind-address",
"0.0.0.0",
])
else:
command.extend(
[
"--conf-use-sockets",
# For unix sockets, authentication fails on first connection if this option is not
# set. Workaround for https://bugs.mysql.com/bug.php?id=107291
"--conf-set-option",
"DEFAULT.server_ssl_mode=PREFERRED",
"--conf-skip-tcp",
]
)
command.extend([
"--conf-use-sockets",
# For unix sockets, authentication fails on first connection if this option is not
# set. Workaround for https://bugs.mysql.com/bug.php?id=107291
"--conf-set-option",
"DEFAULT.server_ssl_mode=PREFERRED",
"--conf-skip-tcp",
])
return command

def _update_configured_socket_file_locations(self) -> None:
Expand Down
37 changes: 17 additions & 20 deletions src/mysql_shell/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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=}")
Expand Down
10 changes: 4 additions & 6 deletions src/relations/hacluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,7 @@ def set_vip(self, vip: Optional[str]) -> None:
json_resources = "{}"
json_resource_params = "{}"

self.relation.data[self.charm.unit].update(
{
"json_resources": json_resources,
"json_resource_params": json_resource_params,
}
)
self.relation.data[self.charm.unit].update({
"json_resources": json_resources,
"json_resource_params": json_resource_params,
})
2 changes: 1 addition & 1 deletion src/server_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 13 additions & 17 deletions src/snap.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,23 +218,19 @@ def update_mysql_router_exporter_service(
)

if enabled:
_snap.set(
{
"mysqlrouter-exporter.listen-port": config.listen_port,
"mysqlrouter-exporter.user": config.username,
"mysqlrouter-exporter.password": config.password,
"mysqlrouter-exporter.url": config.url,
"mysqlrouter-exporter.service-name": self._unit_name.replace("/", "-"),
}
)
_snap.set({
"mysqlrouter-exporter.listen-port": config.listen_port,
"mysqlrouter-exporter.user": config.username,
"mysqlrouter-exporter.password": config.password,
"mysqlrouter-exporter.url": config.url,
"mysqlrouter-exporter.service-name": self._unit_name.replace("/", "-"),
})
if tls:
_snap.set(
{
"mysqlrouter.tls-cacert-path": certificate_authority_filename,
"mysqlrouter.tls-cert-path": certificate_filename,
"mysqlrouter.tls-key-path": key_filename,
}
)
_snap.set({
"mysqlrouter.tls-cacert-path": certificate_authority_filename,
"mysqlrouter.tls-cert-path": certificate_filename,
"mysqlrouter.tls-key-path": key_filename,
})
else:
_snap.unset("mysqlrouter.tls-cacert-path")
_snap.unset("mysqlrouter.tls-cert-path")
Expand All @@ -261,7 +257,7 @@ def _run_command(
command: typing.List[str],
*,
timeout: typing.Optional[int],
input: str = None,
input: str = None, # noqa: A002 Match subprocess.run()
) -> str:
try:
output = subprocess.run(
Expand Down
4 changes: 2 additions & 2 deletions src/workload.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions tests/integration/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,14 @@ def juju_has_secrets(mocker: MockerFixture, request):
"""
juju_version = os.environ["LIBJUJU_VERSION_SPECIFIER"].split("/")[0]
if juju_version < "3":
mocker.patch.object(JujuVersion, "has_secrets", new_callable=PropertyMock).return_value = (
False
)
mocker.patch.object(
JujuVersion, "has_secrets", new_callable=PropertyMock
).return_value = False
return False
else:
mocker.patch.object(JujuVersion, "has_secrets", new_callable=PropertyMock).return_value = (
True
)
mocker.patch.object(
JujuVersion, "has_secrets", new_callable=PropertyMock
).return_value = True
return True


Expand Down
9 changes: 4 additions & 5 deletions tests/integration/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ async def execute_queries_against_unit(
username: The MySQL username
password: The MySQL password
queries: A list of queries to execute
port: The port to connect to in order to execute queries
commit: A keyword arg indicating whether there are any writes queries
Returns:
Expand Down Expand Up @@ -120,9 +121,7 @@ async def delete_file_or_directory_in_unit(ops_test: OpsTest, unit_name: str, pa
if path.strip() in ["/", "."]:
return

return_code, _, _ = await ops_test.juju(
"ssh", unit_name, "sudo", "find", path, "-maxdepth", "1", "-delete"
)
await ops_test.juju("ssh", unit_name, "sudo", "find", path, "-maxdepth", "1", "-delete")


async def write_content_to_file_in_unit(
Expand Down Expand Up @@ -192,7 +191,7 @@ async def ls_la_in_unit(ops_test: OpsTest, unit_name: str, directory: str) -> li
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
Returns:
a list of files returned by ls -la
Expand Down Expand Up @@ -403,7 +402,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}"
Expand Down
7 changes: 4 additions & 3 deletions tests/integration/test_log_rotation.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,10 @@ async def test_log_rotation(ops_test: OpsTest, mysql_router_charm_series: str) -
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(
Expand Down
Loading

0 comments on commit 2eefed0

Please sign in to comment.