diff --git a/src/pubtools/pulplib/_impl/client/client.py b/src/pubtools/pulplib/_impl/client/client.py index 2e3f13ff..e9cb50e5 100644 --- a/src/pubtools/pulplib/_impl/client/client.py +++ b/src/pubtools/pulplib/_impl/client/client.py @@ -1083,6 +1083,13 @@ def create_repository(self, repo): body["importer_type_id"] = importer[0]["importer_type_id"] if importer else None body["importer_config"] = importer[0]["config"] if importer else None + # re-key distributor dict keys due to pulp API inconsistency + for item in body["distributors"]: + item["distributor_id"] = item["id"] + item["distributor_config"] = item["config"] + del item["config"] + del item["id"] + def log_existing_repo(exception): if ( getattr(exception, "response", None) is not None @@ -1099,25 +1106,41 @@ def check_repo(repo_on_server): repo_on_server == repo ), "Repo exists on server with unexpected values" except AssertionError: + # non-fatal cases, some fields don't have to be set before repository creation + # but they're typically set automatically after creation call which results in + # inequality between `repo_on_server`` and `repo` + if repo.relative_url is None: + fatal = False + + if any([dist.repo_id is None for dist in repo.distributors]): + fatal = False + + # fatal cases if importer: - for attr in ["type_id", "config"]: - expected = getattr(repo.importer, attr) - current = getattr(repo_on_server.importer, attr) - if expected != current: - LOG.error( - "Repository %s contains wrong importer %s\n" - "\t expected: %s\n" - "\t current: %s\n", - repo_id, - attr, - expected, - current, + fatal = self._check_unit( + "importer", + ["type_id", "config"], + repo.importer, + repo_on_server.importer, + repo.id, + ) + for expected_item in repo.distributors: + for current_item in repo_on_server.distributors: + if expected_item.type_id == current_item.type_id: + fatal = self._check_unit( + "distributor", + ["id", "type_id", "last_publish"], + expected_item, + current_item, + repo.id, ) - LOG.exception( - "Repository %s exists on server and contains unexpected values", - repo_id, - ) - raise + + if fatal: + LOG.exception( + "Repository %s exists on server and contains unexpected values", + repo_id, + ) + raise return f_return(repo_on_server) @@ -1131,3 +1154,22 @@ def check_repo(repo_on_server): out = f_flat_map(out, check_repo) return f_proxy(out) + + @staticmethod + def _check_unit(unit_type, fields, expected_unit, current_unit, repo_id): + fatal = False + for attr in fields: + expected = getattr(expected_unit, attr) + current = getattr(current_unit, attr) + if expected != current: + fatal = True + LOG.error( + f"Repository %s contains wrong {unit_type} %s\n" + "\t expected: %s\n" + "\t current: %s\n", + repo_id, + attr, + expected, + current, + ) + return fatal diff --git a/src/pubtools/pulplib/_impl/client/retry.py b/src/pubtools/pulplib/_impl/client/retry.py index 822da36c..8d4a6fc1 100644 --- a/src/pubtools/pulplib/_impl/client/retry.py +++ b/src/pubtools/pulplib/_impl/client/retry.py @@ -8,7 +8,7 @@ LOG = logging.getLogger("pubtools.pulplib") -ATTEMPTS = int(os.environ.get("PUBTOOLS_PULPLIB_RETRY_ATTEMPTS", "10")) +ATTEMPTS = int(os.environ.get("PUBTOOLS_PULPLIB_RETRY_ATTEMPTS", "1")) SLEEP = float(os.environ.get("PUBTOOLS_PULPLIB_RETRY_SLEEP", "1.0")) MAX_SLEEP = float(os.environ.get("PUBTOOLS_PULPLIB_RETRY_MAX_SLEEP", "120.0")) diff --git a/tests/fake/test_fake_create_repo.py b/tests/fake/test_fake_create_repo.py index 06363e69..7d8d5978 100644 --- a/tests/fake/test_fake_create_repo.py +++ b/tests/fake/test_fake_create_repo.py @@ -1,4 +1,4 @@ -from pubtools.pulplib import FakeController, Repository +from pubtools.pulplib import FakeController, Repository, Distributor def test_create_repository(): @@ -6,11 +6,26 @@ def test_create_repository(): controller = FakeController() client = controller.client - repo_1 = client.create_repository(Repository(id="repo1")) - repo_2 = client.create_repository(Repository(id="repo2")) + repo_1 = client.create_repository( + Repository( + id="repo1", + distributors=[Distributor(id="dist1", type_id="yum_distributor")], + ) + ) + repo_2 = client.create_repository( + Repository( + id="repo2", + distributors=[Distributor(id="dist2", type_id="yum_distributor")], + ) + ) # adding already existing repository has no effect - _ = client.create_repository(Repository(id="repo1")) + _ = client.create_repository( + Repository( + id="repo1", + distributors=[Distributor(id="dist1", type_id="yum_distributor")], + ) + ) # The change should be reflected in the controller, # with two repositories present assert controller.repositories == [repo_1.result(), repo_2.result()] diff --git a/tests/repository/test_yum_repository.py b/tests/repository/test_yum_repository.py index b082139a..458bdb5e 100644 --- a/tests/repository/test_yum_repository.py +++ b/tests/repository/test_yum_repository.py @@ -2,7 +2,14 @@ import logging import requests -from pubtools.pulplib import Repository, YumRepository, DetachedException, YumImporter +import pubtools.pulplib._impl.compat_attr as attr +from pubtools.pulplib import ( + Repository, + YumRepository, + DetachedException, + YumImporter, + Distributor, +) def test_from_data_gives_yum_repository(): @@ -216,7 +223,16 @@ def test_related_repositories_detached_client(): def test_create_repository(client, requests_mocker): - repo = YumRepository(id="yum_repo_new") + repo = YumRepository( + id="yum_repo_new", + distributors=[ + Distributor( + id="fake_id", + type_id="yum_distributor", + relative_url="yum_repo_new/foo/bar", + ) + ], + ) # create request requests_mocker.post( @@ -237,13 +253,44 @@ def test_create_repository(client, requests_mocker): "config": {}, } ], + "distributors": [ + { + "id": "fake_id", + "distributor_type_id": "yum_distributor", + "config": {"relative_url": "yum_repo_new/foo/bar"}, + "repo_id": "yum_repo_new", + } + ], } ], ) - out = client.create_repository(repo) + out = client.create_repository(repo).result() # check return value of create_repository() call - assert out.result() == repo + # relative_url wasn't set before request for `repo` + assert repo.relative_url is None + # but it's set after creation + assert out.relative_url == "yum_repo_new/foo/bar" + + # repo_id of distributor isn't set before creation call + assert repo.distributors[0].repo_id is None + # but it's set after creation + assert out.distributors[0].repo_id == "yum_repo_new" + # evolve original repo object with new values + repo = attr.evolve( + repo, + relative_url="yum_repo_new/foo/bar", + distributors=[ + Distributor( + id="fake_id", + type_id="yum_distributor", + relative_url="yum_repo_new/foo/bar", + repo_id="yum_repo_new", + ) + ], + ) + # and do full assert + assert out == repo hist = requests_mocker.request_history # there should be exactly 2 requests sent - create and search @@ -256,6 +303,14 @@ def test_create_repository(client, requests_mocker): # are automatically added for yum repository assert create_query["importer_type_id"] == "yum_importer" assert create_query["importer_config"] == {} + # check distributor data + assert len(create_query["distributors"]) == 1 + assert create_query["distributors"][0]["distributor_id"] == "fake_id" + assert create_query["distributors"][0]["distributor_type_id"] == "yum_distributor" + assert ( + create_query["distributors"][0]["distributor_config"]["relative_url"] + == "yum_repo_new/foo/bar" + ) # check the search request for created repo search_query = hist[1].json() @@ -331,7 +386,9 @@ def test_create_repository_already_exists(client, requests_mocker, caplog): def test_create_repository_wrong_data(client, requests_mocker, caplog): repo = YumRepository( - id="yum_repo_existing", importer=YumImporter(config={"new": "value"}) + id="yum_repo_existing", + importer=YumImporter(config={"new": "value"}), + distributors=[Distributor(id="test_dist", type_id="yum_distributor")], ) requests_mocker.post( @@ -353,6 +410,13 @@ def test_create_repository_wrong_data(client, requests_mocker, caplog): "config": {"current": "value"}, } ], + "distributors": [ + { + "id": "fake_id", + "distributor_type_id": "yum_distributor", + "config": {}, + } + ], } ], ) @@ -363,6 +427,7 @@ def test_create_repository_wrong_data(client, requests_mocker, caplog): for text in ( "Repository yum_repo_existing already exists", "Repository yum_repo_existing contains wrong importer config", + "Repository yum_repo_existing contains wrong distributor id", "Repository yum_repo_existing exists on server and contains unexpected values", ): assert text in caplog.text