diff --git a/swh/storage/cassandra/storage.py b/swh/storage/cassandra/storage.py --- a/swh/storage/cassandra/storage.py +++ b/swh/storage/cassandra/storage.py @@ -11,7 +11,6 @@ from typing import Any, Dict, List, Iterable, Optional, Union import attr -from deprecated import deprecated from swh.core.api.serializers import msgpack_loads, msgpack_dumps from swh.model.identifiers import parse_swhid, SWHID @@ -786,20 +785,6 @@ self._cql_runner.origin_add_one(origin) return {"origin:add": len(to_add)} - @deprecated("Use origin_add([origin]) instead") - def origin_add_one(self, origin: Origin) -> str: - known_origin = self.origin_get_one(origin.to_dict()) - - if known_origin: - origin_url = known_origin["url"] - else: - self.journal_writer.origin_add([origin]) - - self._cql_runner.origin_add_one(origin) - origin_url = origin.url - - return origin_url - def origin_visit_add(self, visits: Iterable[OriginVisit]) -> Iterable[OriginVisit]: for visit in visits: origin = self.origin_get({"url": visit.origin}) diff --git a/swh/storage/in_memory.py b/swh/storage/in_memory.py --- a/swh/storage/in_memory.py +++ b/swh/storage/in_memory.py @@ -30,8 +30,6 @@ import attr -from deprecated import deprecated - from swh.core.api.serializers import msgpack_loads, msgpack_dumps from swh.model.identifiers import SWHID from swh.model.model import ( @@ -787,7 +785,6 @@ return {"origin:add": added} - @deprecated("Use origin_add([origin]) instead") def origin_add_one(self, origin: Origin) -> str: if origin.url not in self._origins: self.journal_writer.origin_add([origin]) diff --git a/swh/storage/interface.py b/swh/storage/interface.py --- a/swh/storage/interface.py +++ b/swh/storage/interface.py @@ -1080,25 +1080,6 @@ """ ... - @deprecated - @remote_api_endpoint("origin/add") - def origin_add_one(self, origin: Origin) -> str: - """Add origin to the storage - - Args: - origin: dictionary representing the individual origin to add. This - dict has the following keys: - - - type (FIXME: enum TBD): the origin type ('git', 'wget', ...) - - url (bytes): the url the origin points to - - Returns: - the id of the added origin, or of the identical one that already - exists. - - """ - ... - def stat_counters(self): """compute statistics about the number of tuples in various tables diff --git a/swh/storage/retry.py b/swh/storage/retry.py --- a/swh/storage/retry.py +++ b/swh/storage/retry.py @@ -21,7 +21,6 @@ Revision, Release, Snapshot, - Origin, OriginVisit, MetadataAuthority, MetadataFetcher, @@ -105,10 +104,6 @@ def skipped_content_add(self, content: Iterable[SkippedContent]) -> Dict: return self.storage.skipped_content_add(content) - @swh_retry - def origin_add_one(self, origin: Origin) -> str: - return self.storage.origin_add_one(origin) - @swh_retry def origin_visit_add(self, visits: Iterable[OriginVisit]) -> Iterable[OriginVisit]: return self.storage.origin_visit_add(visits) diff --git a/swh/storage/storage.py b/swh/storage/storage.py --- a/swh/storage/storage.py +++ b/swh/storage/storage.py @@ -9,7 +9,6 @@ from collections import defaultdict from contextlib import contextmanager -from deprecated import deprecated from typing import ( Any, Counter, @@ -1107,13 +1106,6 @@ added += 1 return {"origin:add": added} - @deprecated("Use origin_add([origin]) instead") - @timed - @db_transaction() - def origin_add_one(self, origin: Origin, db=None, cur=None) -> str: - self.origin_add([origin]) - return origin.url - @db_transaction(statement_timeout=500) def stat_counters(self, db=None, cur=None): return {k: v for (k, v) in db.stat_counters()} diff --git a/swh/storage/tests/algos/test_origin.py b/swh/storage/tests/algos/test_origin.py --- a/swh/storage/tests/algos/test_origin.py +++ b/swh/storage/tests/algos/test_origin.py @@ -96,7 +96,7 @@ origin_visit = sample_data_model["origin_visit"][0] assert origin_visit.origin == origin.url - swh_storage.origin_add_one(origin) + swh_storage.origin_add([origin]) swh_storage.origin_visit_add([origin_visit])[0] assert origin_visit.type != "unknown" actual_origin_visit = origin_get_latest_visit_status( diff --git a/swh/storage/tests/algos/test_snapshot.py b/swh/storage/tests/algos/test_snapshot.py --- a/swh/storage/tests/algos/test_snapshot.py +++ b/swh/storage/tests/algos/test_snapshot.py @@ -54,7 +54,7 @@ # no snapshot on origin visit so None origin = sample_data_model["origin"][0] - swh_storage.origin_add_one(origin) + swh_storage.origin_add([origin]) origin_visit, origin_visit2 = sample_data_model["origin_visit"][:2] assert origin_visit.origin == origin.url @@ -86,7 +86,7 @@ def test_snapshot_get_latest(swh_storage, sample_data_model): origin = sample_data_model["origin"][0] - swh_storage.origin_add_one(origin) + swh_storage.origin_add([origin]) visit1, visit2 = sample_data_model["origin_visit"][:2] assert visit1.origin == origin.url diff --git a/swh/storage/tests/test_kafka_writer.py b/swh/storage/tests/test_kafka_writer.py --- a/swh/storage/tests/test_kafka_writer.py +++ b/swh/storage/tests/test_kafka_writer.py @@ -52,7 +52,7 @@ elif obj_type in ("origin_visit",): for obj in objs: assert isinstance(obj, OriginVisit) - storage.origin_add_one(Origin(url=obj.origin)) + storage.origin_add([Origin(url=obj.origin)]) method([obj]) expected_messages += 1 + 1 # 1 visit + 1 visit status else: diff --git a/swh/storage/tests/test_retry.py b/swh/storage/tests/test_retry.py --- a/swh/storage/tests/test_retry.py +++ b/swh/storage/tests/test_retry.py @@ -271,78 +271,13 @@ assert mock_memory.call_count == 1 -def test_retrying_proxy_swh_storage_origin_add_one(swh_storage, sample_data_model): - """Standard origin_add_one works as before - - """ - sample_origin = sample_data_model["origin"][0] - sample_origin_dict = sample_origin.to_dict() - - origin = swh_storage.origin_get(sample_origin_dict) - assert not origin - - swh_storage.origin_add_one(sample_origin) - - origin = swh_storage.origin_get(sample_origin_dict) - assert origin["url"] == sample_origin.url - - -def test_retrying_proxy_swh_storage_origin_add_one_retry( - monkeypatch_sleep, swh_storage, sample_data_model, mocker, fake_hash_collision -): - """Multiple retries for hash collision and psycopg2 error but finally ok - - """ - sample_origin = sample_data_model["origin"][1] - mock_memory = mocker.patch("swh.storage.in_memory.InMemoryStorage.origin_add_one") - mock_memory.side_effect = [ - # first try goes ko - fake_hash_collision, - # second try goes ko - psycopg2.IntegrityError("origin already inserted"), - # ok then! - sample_origin.url, - ] - sample_origin_dict = sample_origin.to_dict() - - origin = swh_storage.origin_get(sample_origin_dict) - assert not origin - - swh_storage.origin_add_one(sample_origin) - - mock_memory.assert_has_calls( - [call(sample_origin), call(sample_origin), call(sample_origin),] - ) - - -def test_retrying_proxy_swh_storage_origin_add_one_failure( - swh_storage, sample_data_model, mocker -): - """Unfiltered errors are raising without retry - - """ - mock_memory = mocker.patch("swh.storage.in_memory.InMemoryStorage.origin_add_one") - mock_memory.side_effect = StorageArgumentException("Refuse to add origin always!") - - sample_origin = sample_data_model["origin"][0] - sample_origin_dict = sample_origin.to_dict() - - origin = swh_storage.origin_get(sample_origin_dict) - assert not origin - - with pytest.raises(StorageArgumentException, match="Refuse to add"): - swh_storage.origin_add_one(sample_origin) - - assert mock_memory.call_count == 1 - - def test_retrying_proxy_swh_storage_origin_visit_add(swh_storage, sample_data_model): """Standard origin_visit_add works as before """ origin = sample_data_model["origin"][0] - swh_storage.origin_add_one(origin) + swh_storage.origin_add([origin]) origins = list(swh_storage.origin_visit_get(origin.url)) assert not origins @@ -364,7 +299,7 @@ """ origin = sample_data_model["origin"][1] - swh_storage.origin_add_one(origin) + swh_storage.origin_add([origin]) mock_memory = mocker.patch("swh.storage.in_memory.InMemoryStorage.origin_visit_add") visit = OriginVisit(origin=origin.url, date=date_visit1, type="git") @@ -578,7 +513,7 @@ """ ori_meta = sample_data_model["origin_metadata"][0] - swh_storage_validate.origin_add_one({"url": ori_meta.id}) + swh_storage_validate.origin_add([{"url": ori_meta.id}]) swh_storage_validate.metadata_authority_add([sample_data_model["authority"][0]]) swh_storage_validate.metadata_fetcher_add([sample_data_model["fetcher"][0]]) @@ -607,7 +542,7 @@ """ ori_meta = sample_data_model["origin_metadata"][0] - swh_storage_validate.origin_add_one({"url": ori_meta.id}) + swh_storage_validate.origin_add([{"url": ori_meta.id}]) swh_storage_validate.metadata_authority_add([sample_data_model["authority"][0]]) swh_storage_validate.metadata_fetcher_add([sample_data_model["fetcher"][0]]) mock_memory = mocker.patch( @@ -647,7 +582,7 @@ mock_memory.side_effect = StorageArgumentException("Refuse to add always!") ori_meta = sample_data_model["origin_metadata"][0] - swh_storage_validate.origin_add_one({"url": ori_meta.id}) + swh_storage_validate.origin_add([{"url": ori_meta.id}]) with pytest.raises(StorageArgumentException, match="Refuse to add"): swh_storage_validate.object_metadata_add([ori_meta]) diff --git a/swh/storage/tests/test_storage.py b/swh/storage/tests/test_storage.py --- a/swh/storage/tests/test_storage.py +++ b/swh/storage/tests/test_storage.py @@ -1312,19 +1312,6 @@ release3.id, } - def test_origin_add_one(self, swh_storage): - origin0 = swh_storage.origin_get(data.origin) - assert origin0 is None - - id = swh_storage.origin_add_one(data.origin) - - actual_origin = swh_storage.origin_get({"url": data.origin["url"]}) - assert actual_origin["url"] == data.origin["url"] - - id2 = swh_storage.origin_add_one(data.origin) - - assert id == id2 - def test_origin_add(self, swh_storage): origin0 = swh_storage.origin_get([data.origin])[0] assert origin0 is None @@ -1408,7 +1395,7 @@ def test_origin_get_legacy(self, swh_storage): assert swh_storage.origin_get(data.origin) is None - swh_storage.origin_add_one(data.origin) + swh_storage.origin_add([data.origin]) actual_origin0 = swh_storage.origin_get({"url": data.origin["url"]}) assert actual_origin0["url"] == data.origin["url"] @@ -1416,7 +1403,7 @@ def test_origin_get(self, swh_storage): assert swh_storage.origin_get(data.origin) is None assert swh_storage.origin_get([data.origin]) == [None] - swh_storage.origin_add_one(data.origin) + swh_storage.origin_add([data.origin]) actual_origin0 = swh_storage.origin_get([{"url": data.origin["url"]}]) assert len(actual_origin0) == 1 @@ -1448,7 +1435,7 @@ def test_origin_visit_get_all(self, swh_storage): origin = Origin.from_dict(data.origin) - swh_storage.origin_add_one(origin) + swh_storage.origin_add([origin]) visits = swh_storage.origin_visit_add( [ OriginVisit( @@ -1582,7 +1569,7 @@ def test_origin_get_by_sha1(self, swh_storage): assert swh_storage.origin_get(data.origin) is None - swh_storage.origin_add_one(data.origin) + swh_storage.origin_add([data.origin]) origins = list(swh_storage.origin_get_by_sha1([sha1(data.origin["url"])])) assert len(origins) == 1 @@ -1601,7 +1588,7 @@ found_origins = list(swh_storage.origin_search(data.origin["url"], regexp=True)) assert len(found_origins) == 0 - swh_storage.origin_add_one(data.origin) + swh_storage.origin_add([data.origin]) origin_data = {"url": data.origin["url"]} found_origins = list(swh_storage.origin_search(data.origin["url"])) assert len(found_origins) == 1 @@ -1617,7 +1604,7 @@ del found_origins[0]["id"] assert found_origins[0] == origin_data - swh_storage.origin_add_one(data.origin2) + swh_storage.origin_add([data.origin2]) origin2_data = {"url": data.origin2["url"]} found_origins = list(swh_storage.origin_search(data.origin2["url"])) assert len(found_origins) == 1 @@ -1636,8 +1623,7 @@ assert found_origins[0] == origin2_data def test_origin_search_no_regexp(self, swh_storage): - swh_storage.origin_add_one(data.origin) - swh_storage.origin_add_one(data.origin2) + swh_storage.origin_add([data.origin, data.origin2]) origin = swh_storage.origin_get({"url": data.origin["url"]}) origin2 = swh_storage.origin_get({"url": data.origin2["url"]}) @@ -1660,8 +1646,7 @@ assert found_origins0 != found_origins1 def test_origin_search_regexp_substring(self, swh_storage): - swh_storage.origin_add_one(data.origin) - swh_storage.origin_add_one(data.origin2) + swh_storage.origin_add([data.origin, data.origin2]) origin = swh_storage.origin_get({"url": data.origin["url"]}) origin2 = swh_storage.origin_get({"url": data.origin2["url"]}) @@ -1688,8 +1673,7 @@ assert found_origins0 != found_origins1 def test_origin_search_regexp_fullstring(self, swh_storage): - swh_storage.origin_add_one(data.origin) - swh_storage.origin_add_one(data.origin2) + swh_storage.origin_add([data.origin, data.origin2]) origin = swh_storage.origin_get({"url": data.origin["url"]}) origin2 = swh_storage.origin_get({"url": data.origin2["url"]}) @@ -1717,7 +1701,7 @@ def test_origin_visit_add(self, swh_storage): origin1 = Origin.from_dict(data.origin2) - swh_storage.origin_add_one(origin1) + swh_storage.origin_add([origin1]) date_visit = now() date_visit2 = date_visit + datetime.timedelta(minutes=1) @@ -1950,7 +1934,7 @@ def test_origin_visit_find_by_date(self, swh_storage): # given origin = Origin.from_dict(data.origin) - swh_storage.origin_add_one(data.origin) + swh_storage.origin_add([data.origin]) visit1 = OriginVisit( origin=origin.url, date=data.date_visit2, type=data.type_visit1, ) @@ -1997,8 +1981,10 @@ swh_storage.origin_visit_find_by_date("foo", data.date_visit2) def test_origin_visit_get_by(self, swh_storage): - origin_url = swh_storage.origin_add_one(data.origin) - origin_url2 = swh_storage.origin_add_one(data.origin2) + origins = [data.origin, data.origin2] + swh_storage.origin_add(origins) + origin_url, origin_url2 = [o["url"] for o in origins] + visit = OriginVisit( origin=origin_url, date=data.date_visit2, type=data.type_visit2, ) @@ -2083,7 +2069,7 @@ # unknown type origin = Origin.from_dict(data.origin) - swh_storage.origin_add_one(origin) + swh_storage.origin_add([origin]) assert swh_storage.origin_visit_get_latest(origin.url, type="unknown") is None def test_origin_visit_get_latest_filter_type(self, swh_storage): @@ -2091,7 +2077,7 @@ """ origin = Origin.from_dict(data.origin) - swh_storage.origin_add_one(origin) + swh_storage.origin_add([origin]) visit1 = OriginVisit( origin=origin.url, date=data.date_visit1, type=data.type_visit1, ) @@ -2135,7 +2121,7 @@ def test_origin_visit_get_latest(self, swh_storage): origin = Origin.from_dict(data.origin) - swh_storage.origin_add_one(origin) + swh_storage.origin_add([origin]) visit1 = OriginVisit( origin=origin.url, date=data.date_visit1, type=data.type_visit1, ) @@ -2281,7 +2267,7 @@ def test_origin_visit_status_get_latest(self, swh_storage): origin1 = Origin.from_dict(data.origin) - swh_storage.origin_add_one(data.origin) + swh_storage.origin_add([data.origin]) # to have some reference visits @@ -2408,7 +2394,8 @@ assert revisions[0]["committer"] == revisions[1]["committer"] def test_snapshot_add_get_empty(self, swh_storage): - origin_url = swh_storage.origin_add_one(data.origin) + swh_storage.origin_add([data.origin]) + origin_url = data.origin["url"] ov1 = swh_storage.origin_visit_add( [ OriginVisit( @@ -2474,7 +2461,7 @@ def test_snapshot_add_get_complete(self, swh_storage): origin_url = data.origin["url"] - origin_url = swh_storage.origin_add_one(data.origin) + swh_storage.origin_add([data.origin]) visit = OriginVisit( origin=origin_url, date=data.date_visit1, type=data.type_visit1, ) @@ -2642,7 +2629,8 @@ assert snapshot == expected_snapshot def test_snapshot_add_get_filtered(self, swh_storage): - origin_url = swh_storage.origin_add_one(data.origin) + swh_storage.origin_add([data.origin]) + origin_url = data.origin["url"] visit = OriginVisit( origin=origin_url, date=data.date_visit1, type=data.type_visit1, ) @@ -2792,7 +2780,8 @@ assert alias1 in branches def test_snapshot_add_get(self, swh_storage): - origin_url = swh_storage.origin_add_one(data.origin) + swh_storage.origin_add([data.origin]) + origin_url = data.origin["url"] visit = OriginVisit( origin=origin_url, date=data.date_visit1, type=data.type_visit1, ) @@ -2822,7 +2811,8 @@ assert origin_visit_info["snapshot"] == data.snapshot["id"] def test_snapshot_add_twice__by_origin_visit(self, swh_storage): - origin_url = swh_storage.origin_add_one(data.origin) + swh_storage.origin_add([data.origin]) + origin_url = data.origin["url"] ov1 = swh_storage.origin_visit_add( [ OriginVisit( @@ -2975,7 +2965,8 @@ # Add other objects. Check their counter increased as well. - origin_url = swh_storage.origin_add_one(data.origin2) + swh_storage.origin_add([data.origin2]) + origin_url = data.origin2["url"] visit = OriginVisit( origin=origin_url, date=data.date_visit2, type=data.type_visit2, ) @@ -4036,7 +4027,7 @@ obj = obj.to_dict() if obj_type == "origin_visit": origin_url = obj.pop("origin") - swh_storage.origin_add_one({"url": origin_url}) + swh_storage.origin_add([{"url": origin_url}]) if "visit" in obj: del obj["visit"] visit = OriginVisit( diff --git a/swh/storage/validate.py b/swh/storage/validate.py --- a/swh/storage/validate.py +++ b/swh/storage/validate.py @@ -6,8 +6,6 @@ import contextlib from typing import Dict, Iterable, Iterator, Optional, Tuple, Type, TypeVar, Union -from deprecated import deprecated - from swh.model.model import ( SkippedContent, Content, @@ -143,10 +141,6 @@ def origin_add(self, origins: Iterable[Union[Dict, Origin]]) -> Dict[str, int]: return self.storage.origin_add([dict_converter(Origin, o) for o in origins]) - @deprecated("Use origin_add([origin]) instead") - def origin_add_one(self, origin: Union[Dict, Origin]) -> int: - return self.storage.origin_add_one(dict_converter(Origin, origin)) - def clear_buffers(self, object_types: Optional[Iterable[str]] = None) -> None: return self.storage.clear_buffers(object_types)