Skip to content

Commit

Permalink
fix(storage): fix async issues
Browse files Browse the repository at this point in the history
  • Loading branch information
frgfm committed Aug 24, 2024
1 parent e36535e commit 6adbd6a
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 26 deletions.
4 changes: 2 additions & 2 deletions src/app/api/api_v1/endpoints/detections.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ async def create_detection(
bucket_name = s3_service.resolve_bucket_name(token_payload.organization_id)
bucket = s3_service.get_bucket(bucket_name)
# Upload the file
if not (await bucket.upload_file(bucket_key, file.file)): # type: ignore[arg-type]
if not bucket.upload_file(bucket_key, file.file): # type: ignore[arg-type]
raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail="Failed upload")
logging.info(f"File uploaded to bucket {bucket_name} with key {bucket_key}.")

Expand Down Expand Up @@ -213,5 +213,5 @@ async def delete_detection(
detection = cast(Detection, await detections.get(detection_id, strict=True))
camera = cast(Camera, await cameras.get(detection.camera_id, strict=True))
bucket = s3_service.get_bucket(s3_service.resolve_bucket_name(camera.organization_id))
await bucket.delete_file(detection.bucket_key)
bucket.delete_file(detection.bucket_key)
await detections.delete(detection_id)
2 changes: 1 addition & 1 deletion src/app/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ async def init_db() -> None:
else:
organization_id = organization.id
# Create the bucket
await s3_service.create_bucket(s3_service.resolve_bucket_name(organization_id))
s3_service.create_bucket(s3_service.resolve_bucket_name(organization_id))

# Check if admin exists
statement = select(User).where(User.login == settings.SUPERADMIN_LOGIN)
Expand Down
14 changes: 7 additions & 7 deletions src/app/services/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,16 @@ def check_file_existence(self, bucket_key: str) -> bool:
logger.warning(e)
return False

async def upload_file(self, bucket_key: str, file_binary: bytes) -> bool:
def upload_file(self, bucket_key: str, file_binary: bytes) -> bool:
"""Upload a file to bucket and return whether the upload succeeded"""
# https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/s3.html#S3.Bucket.upload_fileobj
await self._s3.upload_fileobj(file_binary, self.name, bucket_key)
self._s3.upload_fileobj(file_binary, self.name, bucket_key)
return True

async def delete_file(self, bucket_key: str) -> None:
def delete_file(self, bucket_key: str) -> None:
"""Remove bucket file and return whether the deletion succeeded"""
# https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/s3.html#S3.Client.delete_object
await self._s3.delete_object(Bucket=self.name, Key=bucket_key)
self._s3.delete_object(Bucket=self.name, Key=bucket_key)

def get_public_url(self, bucket_key: str, url_expiration: int = settings.S3_URL_EXPIRATION) -> str:
"""Generate a temporary public URL for a bucket file"""
Expand Down Expand Up @@ -115,10 +115,10 @@ def __init__(
logger.info(f"S3 connected on {endpoint_url}")
self.proxy_url = proxy_url

async def create_bucket(self, bucket_name: str) -> bool:
def create_bucket(self, bucket_name: str) -> bool:
"""Create a new bucket in S3 storage"""
try:
await self._s3.create_bucket(
self._s3.create_bucket(
Bucket=bucket_name, CreateBucketConfiguration={"LocationConstraint": self._s3.meta.region_name}
)
return True
Expand All @@ -135,7 +135,7 @@ async def delete_bucket(self, bucket_name: str) -> bool:
bucket = S3Bucket(self._s3, bucket_name, self.proxy_url)
try:
await bucket.delete_items()
await self._s3.delete_bucket(Bucket=bucket_name)
self._s3.delete_bucket(Bucket=bucket_name)
return True
except ClientError as e:
logger.warning(e)
Expand Down
29 changes: 13 additions & 16 deletions src/tests/services/test_storage.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import io

import boto3
import pytest
from botocore.exceptions import ClientError, EndpointConnectionError, NoCredentialsError

from app.core.config import settings
from app.services.storage import S3Bucket, S3Service
Expand All @@ -23,15 +24,15 @@
settings.S3_ACCESS_KEY,
settings.S3_SECRET_KEY,
settings.S3_PROXY_URL,
EndpointConnectionError,
ValueError,
),
(
settings.S3_REGION,
settings.S3_ENDPOINT_URL,
None,
None,
settings.S3_PROXY_URL,
NoCredentialsError,
ValueError,
),
(
settings.S3_REGION,
Expand All @@ -48,14 +49,11 @@ async def test_s3_service(region, endpoint_url, access_key, secret_key, proxy_ur
if expected_error is None:
service = S3Service(region, endpoint_url, access_key, secret_key, proxy_url)
assert isinstance(service.resolve_bucket_name(1), str)
num_buckets = len(service._s3.list_buckets()["Buckets"])
# Create random bucket
bucket_name = "dummy-bucket"
await service.create_bucket(bucket_name)
assert len(service._s3.list_buckets()["Buckets"]) == num_buckets + 1
service.create_bucket(bucket_name)
# Delete the bucket
await service.delete_bucket(bucket_name)
assert len(service._s3.list_buckets()["Buckets"]) == num_buckets
else:
with pytest.raises(expected_error):
S3Service(region, endpoint_url, access_key, secret_key, proxy_url)
Expand All @@ -64,36 +62,35 @@ async def test_s3_service(region, endpoint_url, access_key, secret_key, proxy_ur
@pytest.mark.parametrize(
("bucket_name", "proxy_url", "expected_error"),
[
(None, None, ValueError),
("dummy-bucket1", None, ClientError),
(None, None, TypeError),
("dummy-bucket1", None, ValueError),
("dummy-bucket2", settings.S3_PROXY_URL, None),
],
)
@pytest.mark.asyncio
async def test_s3_bucket(bucket_name, proxy_url, expected_error, mock_img):
_session = boto3.Session(settings.S3_ACCESS_KEY, settings.S3_SECRET_KEY, region_name=settings.S3_REGION)
_s3 = _session.client("s3", endpoint_url=settings.S3_ENDPOINT_URL)
if expected_error is None:
await _s3.create_bucket(
Bucket=bucket_name, CreateBucketConfiguration={"LocationConstraint": settings.S3_REGION}
)
_s3.create_bucket(Bucket=bucket_name, CreateBucketConfiguration={"LocationConstraint": settings.S3_REGION})
bucket = S3Bucket(_s3, bucket_name, proxy_url)
bucket_key = "logo.png"
# Create file
assert not bucket.check_file_existence(bucket_key)
await bucket.upload_file(bucket_key, mock_img)
bucket.upload_file(bucket_key, io.BytesIO(mock_img))
assert bucket.check_file_existence(bucket_key)
assert isinstance(bucket.get_file_metadata(bucket_key), dict)
assert bucket.get_public_url(bucket_key).startswith("http://")
# Delete file
await bucket.delete_file(bucket_key)
bucket.delete_file(bucket_key)
assert not bucket.check_file_existence(bucket_key)
# Delete all items
await bucket.upload_file(bucket_key, mock_img)
bucket.upload_file(bucket_key, io.BytesIO(mock_img))
assert bucket.check_file_existence(bucket_key)
await bucket.delete_items()
assert not bucket.check_file_existence(bucket_key)
# Delete the bucket
await _s3.delete_bucket(Bucket=bucket_name)
_s3.delete_bucket(Bucket=bucket_name)
else:
with pytest.raises(expected_error):
S3Bucket(_s3, bucket_name, proxy_url)

0 comments on commit 6adbd6a

Please sign in to comment.