clan-cli: do not crash if a machine being deleted is missing from the inventory
We implement that by actually raising `KeyError` in `inventory.delete_by_path` (as advertised in the docstring), since it makes more sense to catch a `KeyError` than a generic `ClanError`.
This commit is contained in:
@@ -431,7 +431,7 @@ def delete_by_path(d: dict[str, Any], path: str) -> Any:
|
|||||||
"""
|
"""
|
||||||
if not path:
|
if not path:
|
||||||
msg = "Cannot delete. Path is empty."
|
msg = "Cannot delete. Path is empty."
|
||||||
raise ClanError(msg)
|
raise KeyError(msg)
|
||||||
|
|
||||||
keys = path.split(".")
|
keys = path.split(".")
|
||||||
current = d
|
current = d
|
||||||
@@ -439,17 +439,17 @@ def delete_by_path(d: dict[str, Any], path: str) -> Any:
|
|||||||
# Navigate to the parent dictionary of the final key
|
# Navigate to the parent dictionary of the final key
|
||||||
for key in keys[:-1]:
|
for key in keys[:-1]:
|
||||||
if key not in current or not isinstance(current[key], dict):
|
if key not in current or not isinstance(current[key], dict):
|
||||||
msg = f"Cannot delete. Key '{path}' not found or not a dictionary '{d}'."
|
msg = f"Cannot delete. Key '{path}' not found or not a dictionary '{d}'"
|
||||||
raise ClanError(msg)
|
raise KeyError(msg)
|
||||||
current = current[key]
|
current = current[key]
|
||||||
|
|
||||||
# Attempt to pop the final key
|
# Attempt to pop the final key
|
||||||
last_key = keys[-1]
|
last_key = keys[-1]
|
||||||
try:
|
try:
|
||||||
value = current.pop(last_key)
|
value = current.pop(last_key)
|
||||||
except KeyError:
|
except KeyError as exc:
|
||||||
msg = f"Cannot delete. Path '{path}' not found in data '{d}'. "
|
msg = f"Cannot delete. Path '{path}' not found in data '{d}'"
|
||||||
raise ClanError(msg) from KeyError
|
raise KeyError(msg) from exc
|
||||||
else:
|
else:
|
||||||
return {last_key: value}
|
return {last_key: value}
|
||||||
|
|
||||||
|
|||||||
@@ -28,7 +28,16 @@ log = logging.getLogger(__name__)
|
|||||||
|
|
||||||
@API.register
|
@API.register
|
||||||
def delete_machine(flake: Flake, name: str) -> None:
|
def delete_machine(flake: Flake, name: str) -> None:
|
||||||
inventory.delete(str(flake.path), {f"machines.{name}"})
|
try:
|
||||||
|
inventory.delete(str(flake.path), {f"machines.{name}"})
|
||||||
|
except KeyError as exc:
|
||||||
|
# louis@(2025-03-09): test infrastructure does not seem to set the
|
||||||
|
# inventory properly, but more importantly only one machine in my
|
||||||
|
# personal clan ended up in the inventory for some reason, so I think
|
||||||
|
# it makes sense to eat the exception here.
|
||||||
|
log.warning(
|
||||||
|
f"{name} was missing or already deleted from the machines inventory: {exc}"
|
||||||
|
)
|
||||||
|
|
||||||
changed_paths: list[Path] = []
|
changed_paths: list[Path] = []
|
||||||
|
|
||||||
|
|||||||
@@ -557,7 +557,7 @@ def test_delete_top_level() -> None:
|
|||||||
def test_delete_key_not_found() -> None:
|
def test_delete_key_not_found() -> None:
|
||||||
data = {"foo": {"bar": 1}}
|
data = {"foo": {"bar": 1}}
|
||||||
# Trying to delete a non-existing key "foo.baz"
|
# Trying to delete a non-existing key "foo.baz"
|
||||||
with pytest.raises(ClanError) as excinfo:
|
with pytest.raises(KeyError) as excinfo:
|
||||||
delete_by_path(data, "foo.baz")
|
delete_by_path(data, "foo.baz")
|
||||||
assert "Cannot delete. Path 'foo.baz'" in str(excinfo.value)
|
assert "Cannot delete. Path 'foo.baz'" in str(excinfo.value)
|
||||||
# Data should remain unchanged
|
# Data should remain unchanged
|
||||||
@@ -567,7 +567,7 @@ def test_delete_key_not_found() -> None:
|
|||||||
def test_delete_intermediate_not_dict() -> None:
|
def test_delete_intermediate_not_dict() -> None:
|
||||||
data = {"foo": "not a dict"}
|
data = {"foo": "not a dict"}
|
||||||
# Trying to go deeper into a non-dict value
|
# Trying to go deeper into a non-dict value
|
||||||
with pytest.raises(ClanError) as excinfo:
|
with pytest.raises(KeyError) as excinfo:
|
||||||
delete_by_path(data, "foo.bar")
|
delete_by_path(data, "foo.bar")
|
||||||
assert "not found or not a dictionary" in str(excinfo.value)
|
assert "not found or not a dictionary" in str(excinfo.value)
|
||||||
# Data should remain unchanged
|
# Data should remain unchanged
|
||||||
@@ -577,7 +577,7 @@ def test_delete_intermediate_not_dict() -> None:
|
|||||||
def test_delete_empty_path() -> None:
|
def test_delete_empty_path() -> None:
|
||||||
data = {"foo": {"bar": 1}}
|
data = {"foo": {"bar": 1}}
|
||||||
# Attempting to delete with an empty path
|
# Attempting to delete with an empty path
|
||||||
with pytest.raises(ClanError) as excinfo:
|
with pytest.raises(KeyError) as excinfo:
|
||||||
delete_by_path(data, "")
|
delete_by_path(data, "")
|
||||||
# Depending on how you handle empty paths, you might raise an error or handle it differently.
|
# Depending on how you handle empty paths, you might raise an error or handle it differently.
|
||||||
# If you do raise an error, check the message.
|
# If you do raise an error, check the message.
|
||||||
@@ -588,7 +588,7 @@ def test_delete_empty_path() -> None:
|
|||||||
def test_delete_non_existent_path_deep() -> None:
|
def test_delete_non_existent_path_deep() -> None:
|
||||||
data = {"foo": {"bar": {"baz": 123}}}
|
data = {"foo": {"bar": {"baz": 123}}}
|
||||||
# non-existent deep path
|
# non-existent deep path
|
||||||
with pytest.raises(ClanError) as excinfo:
|
with pytest.raises(KeyError) as excinfo:
|
||||||
delete_by_path(data, "foo.bar.qux")
|
delete_by_path(data, "foo.bar.qux")
|
||||||
assert "not found" in str(excinfo.value)
|
assert "not found" in str(excinfo.value)
|
||||||
# Data remains unchanged
|
# Data remains unchanged
|
||||||
|
|||||||
Reference in New Issue
Block a user