Skip to content

Commit

Permalink
add stricter type checking
Browse files Browse the repository at this point in the history
  • Loading branch information
thomasf committed May 5, 2023
1 parent ade7d81 commit 1a2bf20
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 28 deletions.
18 changes: 11 additions & 7 deletions minio_storage/files.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from minio_storage.errors import minio_error

if T.TYPE_CHECKING:
from minio_storage.storage import Storage
from minio_storage.storage import MinioStorage

logger = getLogger("minio_storage")

Expand Down Expand Up @@ -36,8 +36,8 @@ def seek(self, *args, **kwargs) -> bool:


class MinioStorageFile(File):
def __init__(self, name: str, mode: str, storage: "Storage", **kwargs):
self._storage: "Storage" = storage
def __init__(self, name: str, mode: str, storage: "MinioStorage", **kwargs):
self._storage: "MinioStorage" = storage
self.name: str = name
self._mode: str = mode
self._file = None
Expand All @@ -52,7 +52,7 @@ def __init__(
self,
name: str,
mode: str,
storage: "Storage",
storage: "MinioStorage",
max_memory_size: T.Optional[int] = None,
**kwargs,
):
Expand All @@ -66,6 +66,7 @@ def __init__(

@property
def file(self):
obj = None
if self._file is None:
try:
obj = self._storage.client.get_object(
Expand All @@ -78,7 +79,8 @@ def file(self):
raise OSError(f"File {self.name} does not exist")
finally:
try:
obj.release_conn()
if obj:
obj.release_conn()
except Exception as e:
logger.error(str(e))
return self._file
Expand All @@ -104,7 +106,7 @@ def __init__(
self,
name: str,
mode: str,
storage: "Storage",
storage: "MinioStorage",
max_memory_size: T.Optional[int] = None,
**kwargs,
):
Expand All @@ -119,6 +121,7 @@ def __init__(
@property
def file(self):
if self._file is None:
obj = None
try:
obj = self._storage.client.get_object(
self._storage.bucket_name, self.name
Expand All @@ -134,7 +137,8 @@ def file(self):
raise minio_error(f"File {self.name} does not exist", error)
finally:
try:
obj.release_conn()
if obj:
obj.release_conn()
except Exception as e:
logger.error(str(e))
return self._file
Expand Down
7 changes: 1 addition & 6 deletions minio_storage/management/commands/minio.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import argparse
import json
import sys
import typing as T
Expand Down Expand Up @@ -46,12 +45,8 @@ def add_arguments(self, parser):
description="valid subcommands",
# required=True,
)
cmds._parser_class = argparse.ArgumentParser # circumvent Django 1.11 bug

cmds.add_parser(self.CHECK, help="check bucket")

cmds.add_parser(self.CREATE, help="make bucket")

cmds.add_parser(self.DELETE, help="remove an empty bucket")

ls = cmds.add_parser(self.LIST, help="list bucket objects or buckets")
Expand Down Expand Up @@ -141,7 +136,7 @@ def storage(self, options):

# TODO: maybe another way
with patch.object(storage_class, "_init_check", return_value=None):
storage = storage_class()
storage = storage_class() # type: ignore
return storage

def bucket_exists(self, storage, bucket_name):
Expand Down
22 changes: 12 additions & 10 deletions minio_storage/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from django.core.files.storage import Storage
from django.utils import timezone
from django.utils.deconstruct import deconstructible
from minio.datatypes import Object

from minio_storage.errors import minio_error
from minio_storage.files import ReadOnlySpooledTemporaryFile
Expand Down Expand Up @@ -106,12 +107,9 @@ def _create_base_url_client(client: minio.Minio, bucket_name: str, base_url: str
base_url_parts = urlsplit(base_url)

# Clone from the normal client, but with base_url as the endpoint
credentials = client._provider.retrieve()
base_url_client = minio.Minio(
base_url_parts.netloc,
access_key=credentials.access_key,
secret_key=credentials.secret_key,
session_token=credentials.session_token,
credentials=client._provider,
secure=base_url_parts.scheme == "https",
# The bucket region may be auto-detected by client (via an HTTP
# request), so don't just use client._region
Expand Down Expand Up @@ -250,8 +248,8 @@ def listdir(self, path: str) -> T.Tuple[T.List, T.List]:

def size(self, name: str) -> int:
try:
info = self.client.stat_object(self.bucket_name, name)
return info.size
info: Object = self.client.stat_object(self.bucket_name, name)
return info.size # type: ignore
except merr.InvalidResponseError as error:
raise minio_error(f"Could not access file size for {name}", error)

Expand Down Expand Up @@ -293,7 +291,7 @@ def _presigned_url(

def url(
self, name: str, *args, max_age: T.Optional[datetime.timedelta] = None
) -> T.Optional[str]:
) -> str:
url = ""
if self.presign_urls:
url = self._presigned_url(name, max_age=max_age)
Expand All @@ -317,7 +315,9 @@ def strip_end(path):
self.bucket_name,
quote(strip_beg(name)),
)
return url
if url:
return url
raise OSError(f"could not produce URL for {name}")

@property
def endpoint_url(self):
Expand All @@ -337,12 +337,14 @@ def created_time(self, name: str) -> datetime.datetime:

def modified_time(self, name: str) -> datetime.datetime:
try:
info = self.client.stat_object(self.bucket_name, name)
return info.last_modified
info: Object = self.client.stat_object(self.bucket_name, name)
if info.last_modified:
return info.last_modified # type: ignore
except merr.InvalidResponseError as error:
raise minio_error(
f"Could not access modification time for file {name}", error
)
raise OSError(f"Could not access modification time for file {name}")


_NoValue = object()
Expand Down
5 changes: 3 additions & 2 deletions tests/test_app/tests/custom_storage_class_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,14 +96,15 @@ def test_custom_storage(self):
) as storage_file:
# Copy the stream from the http stream to the out_file
#
shutil.copyfileobj(storage_file.file, out_file)
shutil.copyfileobj(storage_file.file, out_file) # type: ignore

#
# We are not using the ReadOnlyMinioObjectFile type so we can't seek in
# it.
#
with self.assertRaises(io.UnsupportedOperation):
storage_file.file.seek()
if storage_file.file:
storage_file.file.seek(0)

workspace_files = os.listdir(workspace)
print(workspace_files) # prints: ['secret.txt']
Expand Down
4 changes: 2 additions & 2 deletions tests/test_app/tests/upload_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def test_metadata(self):
"X-Xss-Protection",
"Date",
}
self.assertTrue(metadata_attrs.issubset(res.metadata.keys()))
self.assertTrue(metadata_attrs.issubset(res.metadata.keys())) # type: ignore


@override_settings(
Expand All @@ -105,4 +105,4 @@ def test_default_metadata(self):
self.media_storage.bucket_name, ivan
)

self.assertEqual(res.metadata["Cache-Control"], "max-age=1000")
self.assertEqual(res.metadata["Cache-Control"], "max-age=1000") # type: ignore
2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ deps =
django-stubs==4.2.0
-rdev-requirements.txt
commands =
pyright --skipunannotated --level WARNING
pyright --level WARNING


[testenv:lint]
Expand Down

0 comments on commit 1a2bf20

Please sign in to comment.