Skip to content

Commit efe5a64

Browse files
committed
Address issues from PR feedback
1 parent 72bd345 commit efe5a64

File tree

4 files changed

+32
-33
lines changed

4 files changed

+32
-33
lines changed

README.rst

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -200,13 +200,14 @@ Using optimized S3Boto3Storage
200200
------------------------------
201201

202202
Since ``S3Boto3Storage`` supports storing data from any other fileobj,
203-
it uses a very general ``_save`` function. This leads to the
204-
frontend uploading the file to S3 to then move it byte-by-byte.
205-
For large files this leads to additional loading times for the user.
203+
it uses a generalized ``_save`` function. This leads to the frontend uploading
204+
the file to S3 and then copying it byte-by-byte to perform a move operation just
205+
to rename the uploaded object. For large files this leads to additional loading
206+
times for the user.
206207

207208
That's why S3File provides an optimized version of this method at
208209
``storages_optimized.S3OptimizedUploadStorage``. It uses the more efficient
209-
``copy`` given that we know that we only copy from one S3 location to another.
210+
``copy`` method from S3, given that we know that we only copy from one S3 location to another.
210211

211212
.. code:: python
212213

s3file/storages.py

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@
88
from django.core.files.storage import FileSystemStorage, default_storage
99
from django.utils._os import safe_join
1010

11-
from s3file.storages_optimized import S3OptimizedUploadStorage
12-
1311

1412
class S3MockStorage(FileSystemStorage):
1513
@property
@@ -54,27 +52,6 @@ class bucket:
5452
name = "test-bucket"
5553

5654

57-
class S3OptimizedMockStorage(S3OptimizedUploadStorage):
58-
created_objects = {}
59-
60-
def _compress_content(self, content):
61-
return content
62-
63-
class bucket:
64-
name = "test-bucket"
65-
66-
class Object:
67-
def __init__(self, key):
68-
self.key = key
69-
self.copy_from_bucket = None
70-
self.copy_from_key = None
71-
S3OptimizedMockStorage.created_objects[self.key] = self
72-
73-
def copy(self, s3_object, ExtraArgs):
74-
self.copy_from_bucket = s3_object["Bucket"]
75-
self.copy_from_key = s3_object["Key"]
76-
77-
7855
local_dev = isinstance(default_storage, FileSystemStorage)
7956

8057
storage = default_storage if not local_dev else S3MockStorage()

s3file/storages_optimized.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ def _save(self, name, content):
3333
# obj.upload_fileobj(content, ExtraArgs=params) # Disable upload function
3434

3535
if not hasattr(content, "obj") or not hasattr(content.obj, "key"):
36-
raise RuntimeError(
36+
raise TypeError(
3737
"The content object must be a S3 object and contain a valid key."
3838
)
3939

tests/test_storages.py

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,35 @@
11
import pytest
22
from django.core.files.base import ContentFile
33

4-
from s3file import storages
4+
from s3file.storages_optimized import S3OptimizedUploadStorage
5+
6+
7+
class S3OptimizedMockStorage(S3OptimizedUploadStorage):
8+
created_objects = {}
9+
10+
def _compress_content(self, content):
11+
return content
12+
13+
class bucket:
14+
name = "test-bucket"
15+
16+
class Object:
17+
def __init__(self, key):
18+
self.key = key
19+
self.copy_from_bucket = None
20+
self.copy_from_key = None
21+
S3OptimizedMockStorage.created_objects[self.key] = self
22+
23+
def copy(self, s3_object, ExtraArgs):
24+
self.copy_from_bucket = s3_object["Bucket"]
25+
self.copy_from_key = s3_object["Key"]
526

627

728
class TestStorages:
829
url = "/__s3_mock__/"
930

1031
def test_post__save_optimized(self):
11-
storage = storages.S3OptimizedMockStorage()
32+
storage = S3OptimizedMockStorage()
1233
obj = storage.bucket.Object("tmp/s3file/s3_file.txt")
1334

1435
class Content:
@@ -26,7 +47,7 @@ def __init__(self, obj):
2647
assert stored_object.copy_from_key == "tmp/s3file/s3_file.txt"
2748

2849
def test_post__save_optimized_gzip(self):
29-
storage = storages.S3OptimizedMockStorage()
50+
storage = S3OptimizedMockStorage()
3051
obj = storage.bucket.Object("tmp/s3file/s3_file.css")
3152
storage.gzip = True
3253

@@ -45,9 +66,9 @@ def __init__(self, obj):
4566
assert stored_object.copy_from_key == "tmp/s3file/s3_file.css"
4667

4768
def test_post__save_optimized_fail(self):
48-
storage = storages.S3OptimizedMockStorage()
69+
storage = S3OptimizedMockStorage()
4970

50-
with pytest.raises(RuntimeError) as excinfo:
71+
with pytest.raises(TypeError) as excinfo:
5172
storage._save("tmp/s3file/s3_file_copied.txt", ContentFile(b"s3file"))
5273

5374
assert "The content object must be a S3 object and contain a valid key." in str(

0 commit comments

Comments
 (0)