Prevent file objects unexpectedly being closed by boto after uploading to s3. Fixes #153

Author: madewithkode

tests/storage/test_storage.py

@@ -702,6 +702,22 @@ def test_fetch_file_contents_using_s3_select_raises_errors(
             )
             assert len(file_contents) == 0
 
+    def test_non_closeable_buffered_reader_does_not_close_underlying_file(self):
+        # deliberately not using a context manager to
+        # open/read the file so we can correctly observe
+        # the closing behavior.
+        tmp = tempfile.TemporaryFile(mode="w+b")
+        tmp.write(b"data")
+        tmp.seek(0)
+        try:
+            wrapped = storage.NonCloseableBufferedReader(tmp)
+            wrapped.close()
+
+            # Should have flushed, but NOT closed
+            assert not tmp.closed
+        finally:
+            tmp.close()
+
 
 class TestMemoryFileStore:
     def setup_method(self):

xocto/storage/storage.py

@@ -125,6 +125,15 @@ class ReadableBinaryFile(Protocol):
     def read(self, size: int = ...) -> bytes: ...
 
 
+class NonCloseableBufferedReader(io.BufferedReader):
+    def __init__(self, raw: ReadableBinaryFile, *args: Any, **kwargs: Any):
+        super().__init__(cast(io.RawIOBase, raw), *args, **kwargs)
+
+    def close(self) -> None:
+        # Don't close the underlying file object, just flush the buffer.
+        self.flush()
+
+
 class StreamingBodyIOAdapter(io.RawIOBase):
     """
     Wrapper to adapt a boto3 S3 object to a standard Python "file-like" object
@@ -465,7 +474,7 @@ def store_file(
                 _log_existing_file_returned(key_path)
                 return self.bucket_name, existing_boto_object.key
 
-        readable = _to_stream(contents=contents)
+        readable = NonCloseableBufferedReader(_to_stream(contents=contents))
 
         # `boto_client.upload_fileobj` is type annotated with `Fileobj: BinaryIO`. However, in
         # practice the only file-like method it needs is `read(size=...)`. This cast allows us to
@@ -489,6 +498,11 @@ def store_file(
             ExtraArgs=extra_args,
         )
 
+        # we call readable.detach instead of file_obj.detach because mypy
+        # assumes file_obj is a BinaryIO and not a BufferedReader thus
+        # doesn't have a detach method.
+        readable.detach()
+
         return self.bucket_name, key_path
 
     def store_versioned_file(