Merge pull request #403 from matteofigus/master

Return version_id even if file is empty

Author: martindurant

s3fs/core.py

@@ -718,10 +718,13 @@ def touch(self, path, truncate=True, data=None, **kwargs):
         if not truncate and self.exists(path):
             raise ValueError("S3 does not support touching existent files")
         try:
-            self.call_s3(self.s3.put_object, kwargs, Bucket=bucket, Key=key)
+            write_result = self.call_s3(
+                self.s3.put_object, kwargs, Bucket=bucket, Key=key
+            )
         except ClientError as ex:
             raise translate_boto_error(ex) from ex
         self.invalidate_cache(self._parent(path))
+        return write_result
 
     async def _cat_file(self, path, version_id=None, start=None, end=None):
         bucket, key, vers = self.split_path(path)
@@ -1707,7 +1710,7 @@ def commit(self):
             if self.buffer is not None:
                 logger.debug("Empty file committed %s" % self)
                 self._abort_mpu()
-                self.fs.touch(self.path)
+                write_result = self.fs.touch(self.path)
         elif not self.parts:
             if self.buffer is not None:
                 logger.debug("One-shot upload of %s" % self)
@@ -1720,8 +1723,6 @@ def commit(self):
                     Body=data,
                     **self.kwargs,
                 )
-                if self.fs.version_aware:
-                    self.version_id = write_result.get("VersionId")
             else:
                 raise RuntimeError
         else:
@@ -1734,9 +1735,9 @@ def commit(self):
                 UploadId=self.mpu["UploadId"],
                 MultipartUpload=part_info,
             )
-            if self.fs.version_aware:
-                self.version_id = write_result.get("VersionId")
 
+        if self.fs.version_aware:
+            self.version_id = write_result.get("VersionId")
         # complex cache invalidation, since file's appearance can cause several
         # directories
         self.buffer = None

s3fs/tests/test_s3fs.py

@@ -1544,6 +1544,35 @@ def test_touch(s3):
     assert s3.size(fn) == 4
 
 
+@pytest.mark.skipif(py35, reason="no versions on old moto for py36")
+def test_touch_versions(s3):
+    versioned_file = versioned_bucket_name + "/versioned_file"
+    s3 = S3FileSystem(
+        anon=False, version_aware=True, client_kwargs={"endpoint_url": endpoint_uri}
+    )
+    returned_versions = []
+    with s3.open(versioned_file, "wb") as fo:
+        fo.write(b"1")
+    returned_versions.append(fo.version_id)
+    with s3.open(versioned_file, "wb") as fo:
+        fo.write(b"")
+    returned_versions.append(fo.version_id)
+    assert s3.isfile(versioned_file)
+    versions = s3.object_version_info(versioned_file)
+    version_ids = [version["VersionId"] for version in versions]
+    assert len(version_ids) == 2
+
+    with s3.open(versioned_file) as fo:
+        assert fo.version_id == version_ids[1]
+        assert fo.version_id == returned_versions[1]
+        assert fo.read() == b""
+
+    with s3.open(versioned_file, version_id=version_ids[0]) as fo:
+        assert fo.version_id == version_ids[0]
+        assert fo.version_id == returned_versions[0]
+        assert fo.read() == b"1"
+
+
 def test_cat_missing(s3):
     fn0 = test_bucket_name + "/file0"
     fn1 = test_bucket_name + "/file1"