Skip to content

Commit

Permalink
chore: remove ValueError from RepositoryMapping.add
Browse files Browse the repository at this point in the history
  • Loading branch information
james-garner-canonical committed Dec 5, 2024
1 parent 2b7cb4e commit 709e2c7
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 9 deletions.
18 changes: 14 additions & 4 deletions lib/charms/operator_libs_linux/v0/apt.py
Original file line number Diff line number Diff line change
Expand Up @@ -1428,22 +1428,32 @@ def add( # noqa: D417 # undocumented-param: default_filename intentionally und
"""Add a new repository to the system using add-apt-repository.
Args:
repo: a DebianRepository object where repo.enabled is True
raises:
ValueError: if repo.enabled is False
repo: a DebianRepository object
if repo.enabled is falsey, will return without adding the repository
Raises:
CalledProcessError: if there's an error running apt-add-repository
WARNING: Does not associate the repository with a signing key.
Use `import_key` to add a signing key globally.
WARNING: if repo.enabled is falsey, will return without adding the repository
WARNING: Don't forget to call `apt.update` before installing any packages!
Or call `apt.add_package` with `update_cache=True`.
WARNING: the default_filename keyword argument is provided for backwards compatibility
only. It is not used, and was not used in the previous revision of this library.
"""
if not repo.enabled:
raise ValueError("{repo}.enabled is {value}".format(repo=repo, value=repo.enabled))
logger.warning(
(
"Returning from RepositoryMapping.add(repo=%s) without adding the repo"
" because repo.enabled is %s"
),
repo,
repo.enabled,
)
return
_add_repository(repo)
self._repository_map[_repo_to_identifier(repo)] = repo

Expand Down
13 changes: 8 additions & 5 deletions tests/unit/test_deb822.py
Original file line number Diff line number Diff line change
Expand Up @@ -334,12 +334,15 @@ def test_add_with_deb822(repo_mapping: apt.RepositoryMapping):
# we re-raise CalledProcessError after logging
error = apt.CalledProcessError(1, "cmd")
error.stdout = error.stderr = b""
with patch.object(apt.logger, "error") as mock_logging_error:
with patch.object(apt.subprocess, "run", side_effect=error):
with patch.object(apt.subprocess, "run", side_effect=error):
with patch.object(apt.logger, "error") as mock_error:
with pytest.raises(apt.CalledProcessError):
repo_mapping.add(repo)
mock_logging_error.assert_called_once()
mock_error.assert_called_once()
# call add with a disabled repository
repo._enabled = False
with pytest.raises(ValueError):
repo_mapping.add(repo)
with patch.object(apt, "_add_repository") as mock_add:
with patch.object(apt.logger, "warning") as mock_warning:
repo_mapping.add(repo)
mock_add.assert_not_called()
mock_warning.assert_called_once()

0 comments on commit 709e2c7

Please sign in to comment.