Skip to content

Commit

Permalink
WIP: Add Ruff
Browse files Browse the repository at this point in the history
  • Loading branch information
brianhelba committed Nov 1, 2023
1 parent 85ac3dc commit 9c397fe
Show file tree
Hide file tree
Showing 12 changed files with 107 additions and 74 deletions.
91 changes: 73 additions & 18 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,6 @@ build_dir = "s3_file_field/static/s3_file_field"
[tool.hatch.version]
source = "vcs"

[tool.black]
line-length = 100
target-version = ["py38"]

[tool.coverage.run]
source_pkgs = [
"s3_file_field",
Expand All @@ -122,20 +118,6 @@ source = [
[tool.django-stubs]
django_settings_module = "test_app.settings"

[tool.isort]
profile = "black"
line_length = 100
# Sort by name, don't cluster "from" vs "import"
force_sort_within_sections = true
# Combines "as" imports on the same line
combine_as_imports = true

# These test utilities are local, but are loaded as absolute imports
known_local_folder = [
"fuzzy",
"test_app",
]

[tool.mypy]
files = [
"s3_file_field",
Expand All @@ -156,6 +138,18 @@ mypy_path = [
# as that allows multiple possible import paths
namespace_packages = false

# be strict
disallow_untyped_calls = true
warn_return_any = true
strict_optional = true
warn_no_return = true
warn_redundant_casts = true
warn_unused_ignores = true

disallow_untyped_defs = true
check_untyped_defs = true
no_implicit_reexport = true

[[tool.mypy.overrides]]
module = [
"minio_storage.storage",
Expand Down Expand Up @@ -194,3 +188,64 @@ filterwarnings = [
'ignore:datetime\.datetime\.utcnow:DeprecationWarning:minio',
]
DJANGO_SETTINGS_MODULE = "test_app.settings"

[tool.ruff]
src = [".", "tests"]
line-length = 100
target-version = "py38"
select = ["ALL"]
ignore = [
"ANN", # Annotations
"FIX", # Fixme

# Incompatible with formatter
"ISC001", # Implicitly concatenated string literals on one line

"COM812", # Trailing comma missing
"D100", # Missing docstring in public module
"D101", # Missing docstring in public class
"D102", # Missing docstring in public method
"D103", # Missing docstring in public function
"D104", # Missing docstring in public package
"D105", # Missing docstring in magic method
"D106", # Missing docstring in public nested class
"D107", # Missing docstring in __init__
"TD002", # Missing author in TODO
"TD003", # Missing issue link on the line following this TODO
"TRY003", # Avoid specifying long messages outside the exception class
"EM101", # Exception must not use a string literal, assign to variable first
"EM102", # Exception must not use an f-string literal, assign to variable first

# Maybe
"ERA001", # Found commented-out code
]

[tool.ruff.per-file-ignores]
"example/**" = [
"S105", # Possible hardcoded password
"INP001", # Part of an implicit namespace package.
]
"stubs/**" = [
"FBT", # flake8-boolean-trap
"PLR0913", # Too many arguments to function call
"N", # pep8-naming
]
"tests/**" = [
"S105", # Possible hardcoded password
"SLF001", # Private member accessed
"PLR0913", # Too many arguments to function call
"PLR2004", # Magic value used in comparison
"DJ007", # Do not use `__all__`
"DJ008", # Model does not define `__str__` method
"S101", # Use of assert detected
]
"**/migrations/**" = [
"RUF012", # Mutable class attributes should be annotated with `typing.ClassVar`
]

[tool.ruff.pydocstyle]
convention = "pep257"

[tool.ruff.isort]
# Sort by name, don't cluster "from" vs "import"
force-sort-within-sections = true
4 changes: 2 additions & 2 deletions s3_file_field/_multipart_minio.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def _create_upload_id(
object_key: str,
content_type: str,
) -> str:
return self._client._create_multipart_upload(
return self._client._create_multipart_upload( # noqa: SLF001
bucket_name=self._bucket_name,
object_name=object_key,
headers={
Expand All @@ -37,7 +37,7 @@ def _create_upload_id(
)

def _abort_upload_id(self, object_key: str, upload_id: str) -> None:
self._client._abort_multipart_upload(
self._client._abort_multipart_upload( # noqa: SLF001
bucket_name=self._bucket_name,
object_name=object_key,
upload_id=upload_id,
Expand Down
3 changes: 2 additions & 1 deletion s3_file_field/checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@

@checks.register()
def test_bucket_access(
app_configs: Iterable[AppConfig] | None, **kwargs: Any
app_configs: Iterable[AppConfig] | None,
**kwargs: Any, # noqa: ARG001
) -> list[checks.CheckMessage]:
for storage in iter_storages():
if not MultipartManager.supported_storage(storage):
Expand Down
9 changes: 5 additions & 4 deletions s3_file_field/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,22 +33,23 @@ class S3FileField(FileField):
"A file field which is supports direct uploads to S3 via the "
"UI and fallsback to uploaded to <randomuuid>/filename."
)
_default_max_length = 2000

def __init__(self, *args, **kwargs) -> None:
kwargs.setdefault("max_length", 2000)
kwargs.setdefault("max_length", self._default_max_length)
kwargs.setdefault("upload_to", self.uuid_prefix_filename)
super().__init__(*args, **kwargs)

def deconstruct(self) -> tuple[str, str, Sequence[Any], dict[str, Any]]:
name, path, args, kwargs = super().deconstruct()
if kwargs.get("max_length") == 2000:
if kwargs.get("max_length") == self._default_max_length:
del kwargs["max_length"]
if kwargs.get("upload_to") is self.uuid_prefix_filename:
del kwargs["upload_to"]
return name, path, args, kwargs

@property
def id(self) -> str:
def id(self) -> str: # noqa: A003
"""Return the unique identifier for this field instance."""
if not hasattr(self, "model"):
# TODO: raise a more specific exception
Expand All @@ -66,7 +67,7 @@ def contribute_to_class(self, cls, name, **kwargs):
register_field(self)

@staticmethod
def uuid_prefix_filename(instance: models.Model, filename: str) -> str:
def uuid_prefix_filename(_instance: models.Model, filename: str) -> str:
return f"{uuid4()}/{filename}"

def formfield(
Expand Down
2 changes: 1 addition & 1 deletion s3_file_field/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def __init__(
# widget is a type
if issubclass(widget, AdminFileWidget):
widget = AdminS3FileInput
else:
else: # noqa: PLR5501
# widget is an instance
if isinstance(widget, AdminFileWidget):
# We can't easily re-instantiate the Widget, since we need its initial
Expand Down
5 changes: 3 additions & 2 deletions s3_file_field/rest_framework.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
from __future__ import annotations

from typing import ClassVar

from django.core.files import File
from rest_framework.fields import FileField as FileSerializerField

from s3_file_field.widgets import S3PlaceholderFile


class S3FileSerializerField(FileSerializerField):
default_error_messages = {
default_error_messages: ClassVar[dict[str, str]] = {
"invalid": "Not a valid signed S3 upload. Ensure that the S3 upload flow is correct.",
}

Expand All @@ -25,7 +27,6 @@ def to_internal_value(self, data: str | File) -> str: # type: ignore[override]

# This checks validity of the file name and size
super().to_internal_value(file_object)
assert file_object.name

# fields.S3FileField.save_form_data is not called by DRF, so the same behavior must be
# implemented here
Expand Down
10 changes: 5 additions & 5 deletions s3_file_field/widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import functools
import posixpath
from typing import TYPE_CHECKING, Any, Mapping, NoReturn
from typing import TYPE_CHECKING, Any, ClassVar, Mapping, NoReturn

from django.core import signing
from django.core.files import File
Expand All @@ -28,10 +28,10 @@ class S3PlaceholderFile(File):
size: int

def __init__(self, name: str, size: int) -> None:
self.name = name
super().__init__(file=None, name=name)
self.size = size

def open(self, mode: str | None = None) -> NoReturn:
def open(self, mode: str | None = None) -> NoReturn: # noqa: A003
raise NotImplementedError

def close(self) -> NoReturn:
Expand All @@ -58,8 +58,8 @@ class S3FileInput(ClearableFileInput):
"""Widget to render the S3 File Input."""

class Media:
js = ["s3_file_field/widget.js"]
css = {"all": ["s3_file_field/widget.css"]}
js: ClassVar[list[str]] = ["s3_file_field/widget.js"]
css: ClassVar[dict[str, list[str]]] = {"all": ["s3_file_field/widget.css"]}

def get_context(self, *args, **kwargs) -> dict[str, Any]:
# The base URL cannot be determined at the time the widget is instantiated
Expand Down
1 change: 0 additions & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

from s3_file_field._multipart import MultipartManager
from s3_file_field._sizes import mb

from test_app.models import Resource

# Explicitly load s3_file_field fixtures, late in Pytest plugin load order.
Expand Down
1 change: 0 additions & 1 deletion tests/test_forms.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import pytest

from s3_file_field.forms import S3FormFileField

from test_app.forms import ResourceForm


Expand Down
3 changes: 1 addition & 2 deletions tests/test_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

from s3_file_field import _registry
from s3_file_field.fields import S3FileField

from test_app.models import Resource


Expand Down Expand Up @@ -40,7 +39,7 @@ def test_field_id(s3ff_field: S3FileField) -> None:
def test_field_id_premature() -> None:
s3ff_field = S3FileField()
with pytest.raises(Exception, match=r"contribute_to_class"):
s3ff_field.id
s3ff_field.id # noqa:B018


def test_registry_get_field(s3ff_field: S3FileField) -> None:
Expand Down
18 changes: 9 additions & 9 deletions tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
from django.urls import reverse
import pytest
import requests
from requests.status_codes import codes
from rest_framework.test import APIClient

from s3_file_field._sizes import mb

from fuzzy import FUZZY_UPLOAD_ID, FUZZY_URL, Fuzzy
from s3_file_field._sizes import mb


def test_prepare(api_client: APIClient) -> None:
Expand All @@ -23,7 +23,7 @@ def test_prepare(api_client: APIClient) -> None:
},
format="json",
)
assert resp.status_code == 200
assert resp.status_code == codes.ok
assert resp.data == {
"object_key": Fuzzy(
r"[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}/test.txt"
Expand Down Expand Up @@ -51,7 +51,7 @@ def test_prepare_two_parts(api_client: APIClient) -> None:
},
format="json",
)
assert resp.status_code == 200
assert resp.status_code == codes.ok
assert resp.data == {
"object_key": Fuzzy(
r"[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}/test.txt"
Expand All @@ -77,7 +77,7 @@ def test_prepare_three_parts(api_client: APIClient) -> None:
},
format="json",
)
assert resp.status_code == 200
assert resp.status_code == codes.ok
assert resp.data == {
"object_key": Fuzzy(
r"[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}/test.txt"
Expand Down Expand Up @@ -108,7 +108,7 @@ def test_full_upload_flow(
},
format="json",
)
assert resp.status_code == 200
assert resp.status_code == codes.ok
initialization = resp.data
assert isinstance(initialization, dict)
upload_signature = initialization["upload_signature"]
Expand All @@ -134,7 +134,7 @@ def test_full_upload_flow(
},
format="json",
)
assert resp.status_code == 200
assert resp.status_code == codes.ok
assert resp.data == {
"complete_url": Fuzzy(r".*"),
"body": Fuzzy(r".*"),
Expand All @@ -160,14 +160,14 @@ def test_full_upload_flow(
},
format="json",
)
assert resp.status_code == 200
assert resp.status_code == codes.ok
assert resp.data == {
"field_value": Fuzzy(r".*:.*"),
}

# Verify that the Content headers were stored correctly on the object
object_resp = requests.get(default_storage.url(initialization["object_key"]), timeout=5)
assert resp.status_code == 200
assert resp.status_code == codes.ok
assert object_resp.headers["Content-Type"] == "text/plain"

default_storage.delete(initialization["object_key"])
Loading

0 comments on commit 9c397fe

Please sign in to comment.