Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for Distributor when creating repository #225

Merged
merged 1 commit into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 73 additions & 14 deletions src/pubtools/pulplib/_impl/client/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from more_executors import Executors
from more_executors.futures import f_map, f_flat_map, f_return, f_proxy, f_sequence
from io import StringIO

from ..compat_attr import evolve
from ..model.repository.repo_lock import LOCK_CLAIM_STR
from ..page import Page
from ..criteria import Criteria
Expand Down Expand Up @@ -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
Expand All @@ -1094,25 +1101,58 @@ def log_existing_repo(exception):
raise exception

def check_repo(repo_on_server):
# evolve some fields that 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`
dists = [evolve(item, repo_id=repo_id) for item in repo.distributors]
repo_updated = evolve(
repo, relative_url=repo_on_server.relative_url, distributors=dists
)
try:
assert (
repo_on_server == repo
repo_on_server == repo_updated
), "Repo exists on server with unexpected values"
except AssertionError:
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,
self._check_resource(
"importer",
["type_id", "config"],
repo_updated.importer,
repo_on_server.importer,
repo_updated.id,
)

extra_dist = set()
missing_dist = set()
for expected_item in repo_updated.distributors:
rbikar marked this conversation as resolved.
Show resolved Hide resolved
for current_item in repo_on_server.distributors:
if expected_item.type_id == current_item.type_id:
self._check_resource(
"distributor",
["id", "type_id", "last_publish"],
expected_item,
current_item,
repo_updated.id,
)
extra_dist.discard(current_item.type_id)
missing_dist.discard(expected_item.type_id)
else:
extra_dist.add(current_item.type_id)
missing_dist.add(expected_item.type_id)

for item in extra_dist:
LOG.error(
"Repository %s contains unexpected distributor with type: %s",
repo_id,
item,
)
for item in missing_dist:
LOG.error(
"Repository %s is missing distributor with type: %s",
repo_id,
item,
)

LOG.exception(
"Repository %s exists on server and contains unexpected values",
repo_id,
Expand All @@ -1131,3 +1171,22 @@ def check_repo(repo_on_server):
out = f_flat_map(out, check_repo)

return f_proxy(out)

@staticmethod
def _check_resource(
resource_type, fields, expected_resource, current_resource, repo_id
):
for attr in fields:
expected = getattr(expected_resource, attr)
current = getattr(current_resource, attr)
if expected != current:
LOG.error(
"Repository %s contains wrong %s %s\n"
"\t expected: %s\n"
"\t current: %s\n",
repo_id,
resource_type,
attr,
expected,
current,
)
23 changes: 19 additions & 4 deletions tests/fake/test_fake_create_repo.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,31 @@
from pubtools.pulplib import FakeController, Repository
from pubtools.pulplib import FakeController, Repository, Distributor


def test_create_repository():
"""Client.create_repository() with fake client adds new repositories to controller."""
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()]
85 changes: 80 additions & 5 deletions tests/repository/test_yum_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down Expand Up @@ -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(
Expand All @@ -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
Expand All @@ -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()
Expand Down Expand Up @@ -331,7 +386,12 @@ 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_1", type_id="yum_distributor"),
Distributor(id="test_dist_2", type_id="rpm_rsync_distributor"),
],
)

requests_mocker.post(
Expand All @@ -353,6 +413,18 @@ def test_create_repository_wrong_data(client, requests_mocker, caplog):
"config": {"current": "value"},
}
],
"distributors": [
{
"id": "fake_id_1",
"distributor_type_id": "yum_distributor",
"config": {},
},
{
"id": "fake_id_2",
"distributor_type_id": "iso_distributor",
"config": {},
},
],
}
],
)
Expand All @@ -363,6 +435,9 @@ 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 contains unexpected distributor with type: iso_distributor",
"Repository yum_repo_existing is missing distributor with type: rpm_rsync_distributor",
"Repository yum_repo_existing exists on server and contains unexpected values",
):
assert text in caplog.text
Expand Down
Loading