Skip to content

Commit

Permalink
chore: full-blown ruff config and related fixes/cleanups
Browse files Browse the repository at this point in the history
Introduce a full-blown Ruff config (mostly stolen from Timed) that
can replace the full black+isort+flake8(and plugins) suite that was
used before.

The changes here should not be functional or impact the type system
at all (mypy is still there and watches over us)
  • Loading branch information
winged committed Nov 25, 2024
1 parent 5bd01e5 commit dae30a5
Show file tree
Hide file tree
Showing 17 changed files with 125 additions and 52 deletions.
3 changes: 2 additions & 1 deletion manabi/auth_test.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from collections.abc import Generator
from pathlib import Path
from typing import Generator, cast
from typing import cast
from urllib.parse import quote

import pytest
Expand Down
21 changes: 11 additions & 10 deletions manabi/conftest.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import os
import shutil
from collections.abc import Generator
from pathlib import Path
from subprocess import run
from typing import Any, Dict, Generator
from typing import Any, Dict
from unittest import mock as unitmock

import pytest
Expand Down Expand Up @@ -30,7 +31,7 @@ def configure_hypothesis():
configure_hypothesis()


@pytest.fixture()
@pytest.fixture
def chaos():
with unitmock.patch("manabi.lock.ManabiDbLockStorage", MockManabiDbLockStorage):
yield
Expand All @@ -55,7 +56,7 @@ def clean_db() -> Generator[None, None, None]:
clean_db_exec()


@pytest.fixture()
@pytest.fixture
def postgres_dsn():
return mock._postgres_dsn

Expand All @@ -69,7 +70,7 @@ def lock_storage(request, postgres_dsn):
yield storage


@pytest.fixture()
@pytest.fixture
def write_hooks():
try:
mock._pre_write_hook = "http://127.0.0.1/pre_write_hook"
Expand All @@ -80,7 +81,7 @@ def write_hooks():
mock._post_write_hook = None


@pytest.fixture()
@pytest.fixture
def write_callback():
try:
mock._pre_write_callback = mock.check_token
Expand All @@ -91,7 +92,7 @@ def write_callback():
mock._post_write_callback = None


@pytest.fixture()
@pytest.fixture
def server_dir(s3_file) -> Path:
return mock.get_server_dir()

Expand All @@ -101,19 +102,19 @@ def config(server_dir, lock_storage, request) -> Dict[str, Any]:
return mock.get_config(server_dir, lock_storage, request.param)


@pytest.fixture()
@pytest.fixture
def server(config: Dict[str, Any]) -> Generator:
with mock.run_server(config):
yield


@pytest.fixture()
@pytest.fixture
def mock_now_invalid_past() -> Generator[unitmock.MagicMock, None, None]:
with mock.shift_now(-1200) as m:
yield m


@pytest.fixture()
@pytest.fixture
def mock_now_invalid_future() -> Generator[unitmock.MagicMock, None, None]:
with mock.shift_now(1200) as m:
yield m
Expand All @@ -123,7 +124,7 @@ def mock_now_invalid_future() -> Generator[unitmock.MagicMock, None, None]:
def cargo():
if shutil.which("cargo"):
with mock.branca_impl():
run(["cargo", "run", "x", "y"])
run(["cargo", "run", "x", "y"], check=False)


@pytest.fixture
Expand Down
12 changes: 5 additions & 7 deletions manabi/filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,9 @@ def get_resource_inst(self, path: str, environ: Dict[str, Any]):
token: Token = environ["manabi.token"]
if path.lstrip("/") != str(token.path):
return ManabiFolderResource(path, environ)
else:
path = token.path_as_url()
fp = self._loc_to_file_path(path, environ)
return self.get_file_resource(path, environ, fp)
path = token.path_as_url()
fp = self._loc_to_file_path(path, environ)
return self.get_file_resource(path, environ, fp)


class ManabiS3FileResource(ManabiFileResourceMixin, DAVNonCollection):
Expand Down Expand Up @@ -228,8 +227,7 @@ def get_content(self):
)

def begin_write(self, *, content_type=None):
"""
Open content as a stream for writing.
"""Open content as a stream for writing.
We can't call `super()` here, because we need to use `open` from `smart_open`.
"""
Expand Down Expand Up @@ -260,7 +258,7 @@ def __init__(
super(FilesystemProvider, self).__init__() # type: ignore

if not root_folder:
raise ValueError("Invalid root path: {dir}".format(dir=root_folder))
raise ValueError(f"Invalid root path: {root_folder}")

self.root_folder_path = str(root_folder)
self.readonly = readonly
Expand Down
10 changes: 3 additions & 7 deletions manabi/lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,8 @@ def set_timeout(self, lock):
max_timeout = self.max_timeout
timeout = lock.get("timeout")

if not timeout:
if not timeout or timeout > max_timeout:
lock["timeout"] = max_timeout
else:
if timeout > max_timeout:
lock["timeout"] = max_timeout


class ManabiContextLockMixin(ABC):
Expand Down Expand Up @@ -233,8 +230,7 @@ def __contains__(self, token):
)
if cursor.fetchone() is None:
return False
else:
return True
return True

def __setitem__(self, token, lock):
with self._lock:
Expand Down Expand Up @@ -300,7 +296,7 @@ def execute(self, *args, **kwargs):
try:
self._cursor.execute(*args, **kwargs)
except (InterfaceError, OperationalError):
_logger.warn("Postgres connection lost, reconnecting")
_logger.warning("Postgres connection lost, reconnecting")

self.connect()
self._cursor.execute(*args, **kwargs)
Expand Down
11 changes: 4 additions & 7 deletions manabi/lock_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,19 +187,16 @@ def run_lock_test(req):
res, xml = http("UNLOCK", req, token=token)
if res.status in (200, 204):
return (Results.LOCKED_AND_UNLOCKED, f"status: {res.status}")
else:
return (Results.LOCKED_UNLOCK_FAILED, f"status: {res.status}")
return (Results.LOCKED_UNLOCK_FAILED, f"status: {res.status}")
return (Results.FIRST_LOCK_FAILED, f"status: {res.status}")
except HTTPError as e:
if e.status == 500:
return (Results.SERVER_ERROR, e)
if locked:
return (Results.LOCKED_UNLOCK_FAILED, e)
else:
if e.status == 423:
return (Results.FIRST_LOCK_FAILED, e)
else:
return (Results.UNKNOWN_ERROR, e)
if e.status == 423:
return (Results.FIRST_LOCK_FAILED, e)
return (Results.UNKNOWN_ERROR, e)
except Exception as e:
return (False, e)

Expand Down
2 changes: 0 additions & 2 deletions manabi/log.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,5 +84,3 @@ def __call__(self, environ, start_response):
sub_app_start_response.response_headers,
sub_app_start_response.exc_info,
)

return
9 changes: 4 additions & 5 deletions manabi/mock.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import os
import random
import shutil
from collections.abc import Generator
from contextlib import contextmanager
from functools import partial
from glob import glob
from pathlib import Path
from threading import Thread
from typing import Any, Callable, Dict, Generator, Optional, Tuple, Union
from typing import Any, Callable, Dict, Optional, Tuple, Union
from unittest import mock as unitmock

import requests_mock
Expand Down Expand Up @@ -104,7 +105,7 @@ def get_config(server_dir: Path, lock_storage: Union[Path, str], use_s3: bool =
post_write_hook=_post_write_hook,
post_write_callback=_post_write_callback,
)
provider_class: Union[type[ManabiProvider], type[ManabiS3Provider]] = ManabiProvider
provider_class: type[Union[ManabiProvider, ManabiS3Provider]] = ManabiProvider
provider_kwargs: dict[str, str] = {}
if use_s3:
provider_class = ManabiS3Provider
Expand Down Expand Up @@ -216,9 +217,7 @@ def serve_document(
<a href="http://{base}/dav/{url3}">{path3}</a>
</body>
</html>
""".strip().encode(
"UTF-8"
)
""".strip().encode("UTF-8")

start_response(
"200 Ok",
Expand Down
1 change: 0 additions & 1 deletion manabi/mypy_fix.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ def fix():
(Path(file).absolute().parent / "py.typed").touch()
except Exception as e:
print(e)
pass


if __name__ == "__main__":
Expand Down
3 changes: 1 addition & 2 deletions manabi/token.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,7 @@ def encode(self) -> str:
def from_token(cls, token: "Token", timestamp: Optional[int] = None) -> "Token":
if timestamp is None:
return cls(token.key, token.path, token.payload, now())
else:
return cls(token.key, token.path, token.payload, timestamp)
return cls(token.key, token.path, token.payload, timestamp)

@classmethod
def from_ciphertext(cls, key: Key, ciphertext: str) -> "Token":
Expand Down
2 changes: 1 addition & 1 deletion manabi/token_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ def cargo_build():

@pytest.mark.skipif(cargo_build(), reason="needs rustc and cargo")
def test_other_impl_decode(cargo):
other_impl_decode("hello world".encode("UTF-8"))
other_impl_decode(b"hello world")


# TODO test binary data when branca-rust supports binary data:
Expand Down
3 changes: 1 addition & 2 deletions manabi/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def get_boto_client(
aws_secret_access_key=None,
region_name=None,
):
s3 = boto3.client(
return boto3.client(
"s3",
endpoint_url=endpoint_url
or os.environ.get("S3_ENDPOINT", "http://127.0.0.1:9000"),
Expand All @@ -114,4 +114,3 @@ def get_boto_client(
or os.environ.get("S3_SECRET_ACCESS_KEY", "secretsecret"),
region_name=region_name or os.environ.get("S3_REGION", "us-east-1"),
)
return s3
2 changes: 1 addition & 1 deletion manabi/util_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class CallableClass:


def test_callable():
with pytest.raises(ValueError):
with pytest.raises(ValueError): # noqa: PT011
# well we want to test what happens if it is not correctly typed
CallableClass(True) # type: ignore
CallableClass(test_callable)
3 changes: 1 addition & 2 deletions manabi_django/manabi_django/asgi.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
"""
ASGI config for manabi_django project.
"""ASGI config for manabi_django project.
It exposes the ASGI callable as a module-level variable named ``application``.
Expand Down
3 changes: 1 addition & 2 deletions manabi_django/manabi_django/settings.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
"""
Django settings for manabi_django project.
"""Django settings for manabi_django project.
Generated by 'django-admin startproject' using Django 3.2.14.
Expand Down
3 changes: 1 addition & 2 deletions manabi_django/manabi_django/wsgi.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
"""
WSGI config for manabi_django project.
"""WSGI config for manabi_django project.
It exposes the WSGI callable as a module-level variable named ``application``.
Expand Down
1 change: 1 addition & 0 deletions manabi_django/manage.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#!/usr/bin/env python
"""Django's command-line utility for administrative tasks."""

import os
import sys

Expand Down
88 changes: 88 additions & 0 deletions ruff.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
line-length = 88
exclude = [
"migrations",
"snapshots"
]

[format]
quote-style = "double"
indent-style = "space"
docstring-code-format = true
docstring-code-line-length = 88

[lint]
select = [
"F", # pyflakes
"E", # pycodestyle errors
"I", # isort
"C90", # mccabe
"D", # pydocstyle
"ASYNC", # flake8-async
"B", # flake8-bugbear
"COM", # flake8-commas
"T10", # flake8-debugger
"EXE", # flake8-executable
"ISC", # flake8-implicit-str-concat
"ICN", # flake8-import-conventions
"INP", # flake8-no-pep420
"PIE", # flake8-pie
"PYI", # flake8-pyi
"Q", # flake8-quotes
"RSE", # flake8-raise
"SLOT", # flake8-slots
"TID", # flake8-tidy-imports
"TCH", # flake8-type-checking
"INT", # flake8-gettext
"ERA", # eradicate
"W605", # invalid escape sequence
]
ignore = [
"D203", # we prefer blank-line-before-class (D211) for black compat
"D213", # we prefer multi-line-summary-first-line (D212)
"COM812", # ignore due to conflict with formatter
"ISC001", # ignore due to conflict with formatter
"E501", # managed by formatter
"TD002", # don't require author of TODO
"TD003", # don't require link to TODO
"D100", # don't enforce existance of docstrings
"D101", # don't enforce existance of docstrings
"D102", # don't enforce existance of docstrings
"D103", # don't enforce existance of docstrings
"D104", # don't enforce existance of docstrings
"D105", # don't enforce existance of docstrings
"D106", # don't enforce existance of docstrings
"D107", # don't enforce existance of docstrings
]

[lint.per-file-ignores]
"**/{mock.py,conftest.py,*test*.py}" = [
"D", # pydocstyle is optional for tests
"ANN", # flake8-annotations are optional for tests
"S101", # assert is allow in tests
"S105", # tests may have hardcoded secrets
"S106", # tests may have hardcoded passwords
"S311", # tests may use pseudo-random generators
"S108", # /tmp is allowed in tests since it's expected to be mocked
"DTZ00", # tests often run in UTC
"INP001", # tests do not need a dunder init
"PLR0913", # tests can have a lot of arguments (fixtures)
"PLR2004", # tests can use magic values
]
"**/*/factories.py" = [
"S311", # factories may use pseudo-random generators
]

[lint.isort]
known-first-party = ["timed"]
known-third-party = ["pytest_factoryboy"]
combine-as-imports = true

[lint.flake8-annotations]
# TODO: drop this, its temporary
ignore-fully-untyped = true

[lint.flake8-unused-arguments]
ignore-variadic-names = true

[lint.flake8-pytest-style]
fixture-parentheses = false

0 comments on commit dae30a5

Please sign in to comment.