diff --git a/swh/objstorage/api/client.py b/swh/objstorage/api/client.py --- a/swh/objstorage/api/client.py +++ b/swh/objstorage/api/client.py @@ -30,7 +30,7 @@ reraise_exceptions = [ObjNotFoundError, Error] backend_class = ObjStorageInterface - def restore(self: ObjStorageInterface, content: bytes, obj_id: ObjId): + def restore(self: ObjStorageInterface, content: bytes, obj_id: ObjId) -> None: return self.add(content, obj_id, check_presence=False) def __iter__(self): diff --git a/swh/objstorage/backends/azure.py b/swh/objstorage/backends/azure.py --- a/swh/objstorage/backends/azure.py +++ b/swh/objstorage/backends/azure.py @@ -206,10 +206,10 @@ """ return sum(1 for i in self) - def add(self, content: bytes, obj_id: ObjId, check_presence: bool = True) -> ObjId: + def add(self, content: bytes, obj_id: ObjId, check_presence: bool = True) -> None: """Add an obj in storage if it's not there already.""" if check_presence and obj_id in self: - return obj_id + return hex_obj_id = self._internal_id(obj_id) @@ -228,9 +228,7 @@ # removes the blob, it should be safe to just ignore the error. pass - return obj_id - - def restore(self, content: bytes, obj_id: ObjId): + def restore(self, content: bytes, obj_id: ObjId) -> None: """Restore a content.""" if obj_id in self: self.delete(obj_id) diff --git a/swh/objstorage/backends/http.py b/swh/objstorage/backends/http.py --- a/swh/objstorage/backends/http.py +++ b/swh/objstorage/backends/http.py @@ -55,13 +55,13 @@ def __len__(self): raise exc.NonIterableObjStorage("__len__") - def add(self, content: bytes, obj_id: ObjId, check_presence: bool = True) -> ObjId: + def add(self, content: bytes, obj_id: ObjId, check_presence: bool = True) -> None: raise exc.ReadOnlyObjStorage("add") def delete(self, obj_id: ObjId): raise exc.ReadOnlyObjStorage("delete") - def restore(self, content: bytes, obj_id: ObjId): + def restore(self, content: bytes, obj_id: ObjId) -> None: raise exc.ReadOnlyObjStorage("restore") def list_content( diff --git a/swh/objstorage/backends/in_memory.py b/swh/objstorage/backends/in_memory.py --- a/swh/objstorage/backends/in_memory.py +++ b/swh/objstorage/backends/in_memory.py @@ -28,14 +28,12 @@ def __iter__(self): return iter(sorted(self.state)) - def add(self, content: bytes, obj_id: ObjId, check_presence: bool = True) -> ObjId: + def add(self, content: bytes, obj_id: ObjId, check_presence: bool = True) -> None: if check_presence and obj_id in self: - return obj_id + return self.state[obj_id] = content - return obj_id - def get(self, obj_id: ObjId) -> bytes: if obj_id not in self: raise ObjNotFoundError(obj_id) diff --git a/swh/objstorage/backends/libcloud.py b/swh/objstorage/backends/libcloud.py --- a/swh/objstorage/backends/libcloud.py +++ b/swh/objstorage/backends/libcloud.py @@ -157,14 +157,13 @@ """ return sum(1 for i in self) - def add(self, content: bytes, obj_id: ObjId, check_presence: bool = True) -> ObjId: + def add(self, content: bytes, obj_id: ObjId, check_presence: bool = True) -> None: if check_presence and obj_id in self: - return obj_id + return self._put_object(content, obj_id) - return obj_id - def restore(self, content: bytes, obj_id: ObjId): + def restore(self, content: bytes, obj_id: ObjId) -> None: return self.add(content, obj_id, check_presence=False) def get(self, obj_id: ObjId) -> bytes: diff --git a/swh/objstorage/backends/pathslicing.py b/swh/objstorage/backends/pathslicing.py --- a/swh/objstorage/backends/pathslicing.py +++ b/swh/objstorage/backends/pathslicing.py @@ -227,10 +227,10 @@ content: bytes, obj_id: ObjId, check_presence: bool = True, - ) -> ObjId: + ) -> None: if check_presence and obj_id in self: # If the object is already present, return immediately. - return obj_id + return hex_obj_id = hashutil.hash_to_hex(obj_id) compressor = compressors[self.compression]() @@ -238,8 +238,6 @@ f.write(compressor.compress(content)) f.write(compressor.flush()) - return obj_id - def get(self, obj_id: ObjId) -> bytes: if obj_id not in self: raise ObjNotFoundError(obj_id) diff --git a/swh/objstorage/backends/seaweedfs/objstorage.py b/swh/objstorage/backends/seaweedfs/objstorage.py --- a/swh/objstorage/backends/seaweedfs/objstorage.py +++ b/swh/objstorage/backends/seaweedfs/objstorage.py @@ -74,9 +74,9 @@ """ return sum(1 for i in self) - def add(self, content: bytes, obj_id: ObjId, check_presence: bool = True) -> ObjId: + def add(self, content: bytes, obj_id: ObjId, check_presence: bool = True) -> None: if check_presence and obj_id in self: - return obj_id + return def compressor(data): comp = compressors[self.compression]() @@ -88,9 +88,8 @@ ), "list of content chunks is not supported anymore" self.wf.put(io.BytesIO(b"".join(compressor(content))), self._path(obj_id)) - return obj_id - def restore(self, content: bytes, obj_id: ObjId): + def restore(self, content: bytes, obj_id: ObjId) -> None: return self.add(content, obj_id, check_presence=False) def get(self, obj_id: ObjId) -> bytes: diff --git a/swh/objstorage/backends/winery/objstorage.py b/swh/objstorage/backends/winery/objstorage.py --- a/swh/objstorage/backends/winery/objstorage.py +++ b/swh/objstorage/backends/winery/objstorage.py @@ -38,8 +38,8 @@ def __contains__(self, obj_id): return obj_id in self.winery - def add(self, content: bytes, obj_id: ObjId, check_presence: bool = True) -> ObjId: - return self.winery.add(content, obj_id, check_presence) + def add(self, content: bytes, obj_id: ObjId, check_presence: bool = True) -> None: + self.winery.add(content, obj_id, check_presence) def check(self, obj_id: ObjId) -> None: return self.winery.check(obj_id) diff --git a/swh/objstorage/interface.py b/swh/objstorage/interface.py --- a/swh/objstorage/interface.py +++ b/swh/objstorage/interface.py @@ -65,7 +65,7 @@ ... @remote_api_endpoint("content/add") - def add(self, content: bytes, obj_id: ObjId, check_presence: bool = True) -> ObjId: + def add(self, content: bytes, obj_id: ObjId, check_presence: bool = True) -> None: """Add a new object to the object storage. Args: @@ -95,7 +95,7 @@ """ ... - def restore(self, content: bytes, obj_id: ObjId): + def restore(self, content: bytes, obj_id: ObjId) -> None: """Restore a content that have been corrupted. This function is identical to add but does not check if diff --git a/swh/objstorage/multiplexer/multiplexer_objstorage.py b/swh/objstorage/multiplexer/multiplexer_objstorage.py --- a/swh/objstorage/multiplexer/multiplexer_objstorage.py +++ b/swh/objstorage/multiplexer/multiplexer_objstorage.py @@ -223,7 +223,7 @@ return obj_iterator() - def add(self, content: bytes, obj_id: ObjId, check_presence: bool = True) -> ObjId: + def add(self, content: bytes, obj_id: ObjId, check_presence: bool = True) -> None: """Add a new object to the object storage. If the adding step works in all the storages that accept this content, @@ -243,7 +243,7 @@ always readable as well, any id will be valid to retrieve a content. """ - results = self.wrap_call( + self.wrap_call( self.get_write_threads(obj_id), "add", content, @@ -251,13 +251,6 @@ check_presence=check_presence, ) - for result in results: - if not result: - continue - return result - - assert False, "No backend objstorage configured" - def add_batch(self, contents, check_presence=True) -> Dict: """Add a batch of new objects to the object storage.""" write_threads = list(self.get_write_threads()) @@ -278,7 +271,7 @@ "object:add:bytes": summed["object:add:bytes"] // len(results), } - def restore(self, content: bytes, obj_id: ObjId): + def restore(self, content: bytes, obj_id: ObjId) -> None: return self.wrap_call( self.get_write_threads(obj_id), "restore", diff --git a/swh/objstorage/objstorage.py b/swh/objstorage/objstorage.py --- a/swh/objstorage/objstorage.py +++ b/swh/objstorage/objstorage.py @@ -103,9 +103,9 @@ summary["object:add:bytes"] += len(content) return summary - def restore(self: ObjStorageInterface, content: bytes, obj_id: ObjId): + def restore(self: ObjStorageInterface, content: bytes, obj_id: ObjId) -> None: # check_presence to false will erase the potential previous content. - return self.add(content, obj_id, check_presence=False) + self.add(content, obj_id, check_presence=False) def get_batch( self: ObjStorageInterface, obj_ids: List[ObjId] diff --git a/swh/objstorage/tests/objstorage_testing.py b/swh/objstorage/tests/objstorage_testing.py --- a/swh/objstorage/tests/objstorage_testing.py +++ b/swh/objstorage/tests/objstorage_testing.py @@ -62,23 +62,19 @@ def test_add_get_w_id(self): content, obj_id = self.hash_content(b"add_get_w_id") - r = self.storage.add(content, obj_id=obj_id) - self.assertEqual(obj_id, r) + self.storage.add(content, obj_id=obj_id) self.assertContentMatch(obj_id, content) def test_add_twice(self): content, obj_id = self.hash_content(b"add_twice") - r = self.storage.add(content, obj_id=obj_id) - self.assertEqual(obj_id, r) + self.storage.add(content, obj_id=obj_id) self.assertContentMatch(obj_id, content) - r = self.storage.add(content, obj_id=obj_id, check_presence=False) - self.assertEqual(obj_id, r) + self.storage.add(content, obj_id=obj_id, check_presence=False) self.assertContentMatch(obj_id, content) def test_add_big(self): content, obj_id = self.hash_content(b"add_big" * 1024 * 1024) - r = self.storage.add(content, obj_id=obj_id) - self.assertEqual(obj_id, r) + self.storage.add(content, obj_id=obj_id) self.assertContentMatch(obj_id, content) def test_add_get_batch(self): @@ -101,12 +97,10 @@ valid_content, valid_obj_id = self.hash_content(b"restore_content") invalid_content = b"unexpected content" - id_adding = self.storage.add(invalid_content, valid_obj_id) - self.assertEqual(id_adding, valid_obj_id) + self.storage.add(invalid_content, valid_obj_id) with self.assertRaises(exc.Error): - self.storage.check(id_adding) - id_restore = self.storage.restore(valid_content, valid_obj_id) - self.assertEqual(id_restore, valid_obj_id) + self.storage.check(valid_obj_id) + self.storage.restore(valid_content, valid_obj_id) self.assertContentMatch(valid_obj_id, valid_content) def test_get_missing(self): diff --git a/swh/objstorage/tests/test_objstorage_http.py b/swh/objstorage/tests/test_objstorage_http.py --- a/swh/objstorage/tests/test_objstorage_http.py +++ b/swh/objstorage/tests/test_objstorage_http.py @@ -26,7 +26,9 @@ objids = [] for i in range(100): content = f"some content {i}".encode() - objids.append(sto_back.add(content, obj_id=compute_hash(content))) + obj_id = compute_hash(content) + objids.append(obj_id) + sto_back.add(content, obj_id=obj_id) url = "http://127.0.0.1/content/" sto_front = get_objstorage(cls="http", url=url) @@ -84,12 +86,11 @@ # create an invalid object in the in-memory objstorage invalid_content = b"p0wn3d content" fake_objid = "\x01" * 20 - id_added = sto_back.add(invalid_content, fake_objid) - assert id_added == fake_objid + sto_back.add(invalid_content, fake_objid) # the http objstorage should report it as invalid with pytest.raises(exc.Error): - sto_front.check(id_added) + sto_front.check(fake_objid) def test_http_objstorage_read_only():