From 92f10a712d953fc4bc73fc75f474cbf8256736e7 Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Tue, 12 Sep 2023 05:14:14 +0000 Subject: [PATCH] scuba: Use Path objects in ScubaVolume --- scuba/config.py | 27 ++++++++---------- scuba/scuba.py | 6 ++-- tests/test_config.py | 65 ++++++++------------------------------------ tests/test_scuba.py | 14 +++------- tests/utils.py | 18 ++++++++++++ 5 files changed, 49 insertions(+), 81 deletions(-) diff --git a/scuba/config.py b/scuba/config.py index 913dd971..311963a4 100644 --- a/scuba/config.py +++ b/scuba/config.py @@ -314,7 +314,7 @@ def _get_str(data: CfgData, key: str, default: Optional[str] = None) -> Optional return _get_typed_val(data, key, str, default) -def _get_dict(data: CfgData, key: str) -> Optional[dict]: +def _get_dict(data: CfgData, key: str) -> Optional[dict[str, Any]]: return _get_typed_val(data, key, dict) @@ -323,19 +323,19 @@ def _get_delimited_str_list(data: CfgData, key: str, sep: str) -> List[str]: return s.split(sep) if s else [] -def _get_volumes(data: CfgData) -> Optional[Dict[str, ScubaVolume]]: +def _get_volumes(data: CfgData) -> Optional[Dict[Path, ScubaVolume]]: voldata = _get_dict(data, "volumes") if voldata is None: return None vols = {} - for cpath, v in voldata.items(): - cpath = _expand_path(cpath) + for cpath_str, v in voldata.items(): + cpath = _expand_path(cpath_str) vols[cpath] = ScubaVolume.from_dict(cpath, v) return vols -def _expand_path(in_str: str) -> str: +def _expand_path(in_str: str) -> Path: try: output = expand_env_vars(in_str) except KeyError as ke: @@ -348,17 +348,17 @@ def _expand_path(in_str: str) -> str: f"Unable to expand string '{in_str}' due to parsing errors" ) from ve - return output + return Path(output) @dataclasses.dataclass(frozen=True) class ScubaVolume: - container_path: str - host_path: str # TODO: Optional for anonymous volume + container_path: Path + host_path: Path # TODO: Optional for anonymous volume options: List[str] = dataclasses.field(default_factory=list) @classmethod - def from_dict(cls, cpath: str, node: CfgNode) -> ScubaVolume: + def from_dict(cls, cpath: Path, node: CfgNode) -> ScubaVolume: # Treat a null node as an empty dict if node is None: node = {} @@ -390,10 +390,7 @@ def from_dict(cls, cpath: str, node: CfgNode) -> ScubaVolume: raise ConfigError(f"{cpath}: must be string or dict") def get_vol_opt(self) -> str: - # TODO: change host_path and container_path to Path objects - return make_vol_opt( - Path(self.host_path), Path(self.container_path), self.options - ) + return make_vol_opt(self.host_path, self.container_path, self.options) @dataclasses.dataclass(frozen=True) @@ -406,7 +403,7 @@ class ScubaAlias: shell: Optional[str] = None as_root: bool = False docker_args: Optional[List[str]] = None - volumes: Optional[Dict[str, ScubaVolume]] = None + volumes: Optional[Dict[Path, ScubaVolume]] = None @classmethod def from_dict(cls, name: str, node: CfgNode) -> ScubaAlias: @@ -434,7 +431,7 @@ class ScubaConfig: shell: str entrypoint: Optional[str] docker_args: Optional[List[str]] # TODO: drop Optional? - volumes: Optional[Dict[str, ScubaVolume]] # TODO: drop Optional? Dict? + volumes: Optional[Dict[Path, ScubaVolume]] # TODO: drop Optional? Dict? aliases: Dict[str, ScubaAlias] hooks: Dict[str, List[str]] environment: Environment diff --git a/scuba/scuba.py b/scuba/scuba.py index b47bd26b..0c4f9899 100644 --- a/scuba/scuba.py +++ b/scuba/scuba.py @@ -191,7 +191,7 @@ def try_create_volumes(self) -> None: return for vol in self.context.volumes.values(): - if os.path.exists(vol.host_path): + if vol.host_path.exists(): continue try: @@ -403,7 +403,7 @@ def get_docker_cmdline(self) -> Sequence[str]: class ScubaContext: image: str environment: Dict[str, str] # key: value - volumes: Dict[str, ScubaVolume] + volumes: Dict[Path, ScubaVolume] shell: str docker_args: List[str] script: Optional[List[str]] = None # TODO: drop Optional? @@ -435,7 +435,7 @@ def process_command( entrypoint = cfg.entrypoint environment = copy.copy(cfg.environment) docker_args = copy.copy(cfg.docker_args) or [] - volumes: Dict[str, ScubaVolume] = copy.copy(cfg.volumes or {}) + volumes: Dict[Path, ScubaVolume] = copy.copy(cfg.volumes or {}) as_root = False if command: diff --git a/tests/test_config.py b/tests/test_config.py index efe70d59..8c1bc06a 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -892,9 +892,7 @@ def test_volumes_simple_volume(self) -> None: assert config.volumes is not None assert len(config.volumes) == 1 - v = config.volumes["/cpath"] - assert v.container_path == "/cpath" - assert v.host_path == "/hpath" + assert_vol(config.volumes, "/cpath", "/hpath") def test_volumes_complex(self) -> None: """volumes can be set using the complex form""" @@ -917,23 +915,9 @@ def test_volumes_complex(self) -> None: assert vols is not None assert len(vols) == 3 - v = vols["/foo"] - assert isinstance(v, scuba.config.ScubaVolume) - assert v.container_path == "/foo" - assert v.host_path == "/host/foo" - assert v.options == [] - - v = vols["/bar"] - assert isinstance(v, scuba.config.ScubaVolume) - assert v.container_path == "/bar" - assert v.host_path == "/host/bar" - assert v.options == [] - - v = vols["/snap"] - assert isinstance(v, scuba.config.ScubaVolume) - assert v.container_path == "/snap" - assert v.host_path == "/host/snap" - assert v.options == ["z", "ro"] + assert_vol(vols, "/foo", "/host/foo") + assert_vol(vols, "/bar", "/host/bar") + assert_vol(vols, "/snap", "/host/snap", ["z", "ro"]) def test_alias_volumes_set(self) -> None: """docker_args can be set via alias""" @@ -958,17 +942,8 @@ def test_alias_volumes_set(self) -> None: assert vols is not None assert len(vols) == 2 - v = vols["/foo"] - assert isinstance(v, scuba.config.ScubaVolume) - assert v.container_path == "/foo" - assert v.host_path == "/host/foo" - assert v.options == [] - - v = vols["/bar"] - assert isinstance(v, scuba.config.ScubaVolume) - assert v.container_path == "/bar" - assert v.host_path == "/host/bar" - assert v.options == ["z", "ro"] + assert_vol(vols, "/foo", "/host/foo") + assert_vol(vols, "/bar", "/host/bar", ["z", "ro"]) def test_volumes_with_env_vars_simple(self, monkeypatch) -> None: """volume definitions can contain environment variables""" @@ -988,11 +963,7 @@ def test_volumes_with_env_vars_simple(self, monkeypatch) -> None: assert vols is not None assert len(vols) == 1 - v = list(vols.values())[0] - assert isinstance(v, scuba.config.ScubaVolume) - assert v.container_path == "/bar/baz/foo" - assert v.host_path == "/moo/doo/foo" - assert v.options == [] + assert_vol(vols, "/bar/baz/foo", "/moo/doo/foo") def test_volumes_with_env_vars_complex(self, monkeypatch) -> None: """complex volume definitions can contain environment variables""" @@ -1019,23 +990,11 @@ def test_volumes_with_env_vars_complex(self, monkeypatch) -> None: assert vols is not None assert len(vols) == 3 - v = vols["/home/testuser/.config"] - assert isinstance(v, scuba.config.ScubaVolume) - assert v.container_path == "/home/testuser/.config" - assert v.host_path == "/home/testuser/.config" - assert v.options == [] - - v = vols["/tmp/"] - assert isinstance(v, scuba.config.ScubaVolume) - assert v.container_path == "/tmp/" - assert v.host_path == "/home/testuser/scuba/myproject/tmp" - assert v.options == [] - - v = vols["/var/spool/mail/container"] - assert isinstance(v, scuba.config.ScubaVolume) - assert v.container_path == "/var/spool/mail/container" - assert v.host_path == "/var/spool/mail/testuser" - assert v.options == ["z", "ro"] + assert_vol(vols, "/home/testuser/.config", "/home/testuser/.config") + assert_vol(vols, "/tmp", "/home/testuser/scuba/myproject/tmp") + assert_vol( + vols, "/var/spool/mail/container", "/var/spool/mail/testuser", ["z", "ro"] + ) def test_volumes_with_invalid_env_vars(self, monkeypatch) -> None: """Volume definitions cannot include unset env vars""" diff --git a/tests/test_scuba.py b/tests/test_scuba.py index ab485939..4aff2e6a 100644 --- a/tests/test_scuba.py +++ b/tests/test_scuba.py @@ -1,5 +1,6 @@ import pytest import shlex +from .utils import assert_vol from scuba.config import ScubaConfig, ConfigError, OverrideStr from scuba.scuba import ScubaContext @@ -340,13 +341,8 @@ def test_process_command_alias_extends_volumes(self) -> None: vols = result.volumes assert len(vols) == 2 - v = vols["/foo"] - assert v.container_path == "/foo" - assert v.host_path == "/host/foo" - - v = vols["/bar"] - assert v.container_path == "/bar" - assert v.host_path == "/host/bar" + assert_vol(vols, "/foo", "/host/foo") + assert_vol(vols, "/bar", "/host/bar") def test_process_command_alias_updates_volumes(self) -> None: """aliases can extend the volumes""" @@ -370,6 +366,4 @@ def test_process_command_alias_updates_volumes(self) -> None: vols = result.volumes assert len(vols) == 1 - v = vols["/foo"] - assert v.container_path == "/foo" - assert v.host_path == "/alternate/foo" + assert_vol(vols, "/foo", "/alternate/foo") diff --git a/tests/utils.py b/tests/utils.py index 66fe174c..37b3218e 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -1,6 +1,7 @@ import os import sys from os.path import normpath +from pathlib import Path import tempfile import shutil import unittest @@ -9,6 +10,8 @@ from typing import Any, Sequence, Union from unittest import mock +from scuba.config import ScubaVolume + PathStr = Union[Path, str] @@ -26,6 +29,21 @@ def assert_str_equalish(exp: Any, act: Any) -> None: assert exp == act +def assert_vol( + vols: dict[Path, ScubaVolume], + cpath_str: str, + hpath_str: str, + options: list[str] = [], +) -> None: + cpath = Path(cpath_str) + hpath = Path(hpath_str) + v = vols[cpath] + assert isinstance(v, ScubaVolume) + assert v.container_path == cpath + assert v.host_path == hpath + assert v.options == options + + def make_executable(path: PathStr) -> None: mode = os.stat(path).st_mode mode |= (mode & 0o444) >> 2 # copy R bits to X