From f022435e3c4ea57cc9834bb4e555ad2b0a92e1a5 Mon Sep 17 00:00:00 2001 From: Tom Close Date: Mon, 18 Mar 2024 09:28:12 +1100 Subject: [PATCH 1/3] use exist_ok instead of explicit check for path exists when creating hash cache so it is multiprocess safe --- pydra/utils/hash.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pydra/utils/hash.py b/pydra/utils/hash.py index 90e132d1e..1ce35251c 100644 --- a/pydra/utils/hash.py +++ b/pydra/utils/hash.py @@ -60,8 +60,7 @@ def location_converter(path: ty.Union[Path, str, None]) -> Path: if path is None: path = PersistentCache.location_default() path = Path(path) - if not path.exists(): - path.mkdir(parents=True) + path.mkdir(parents=True, exist_ok=True) return path From ae9269db94b4ce1385082a345997afbf501af791 Mon Sep 17 00:00:00 2001 From: Tom Close Date: Mon, 18 Mar 2024 09:39:56 +1100 Subject: [PATCH 2/3] changed expected exception when cache dir is a file to FileExistsError --- pydra/utils/hash.py | 7 ------- pydra/utils/tests/test_hash.py | 2 +- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/pydra/utils/hash.py b/pydra/utils/hash.py index 1ce35251c..4d8042cb3 100644 --- a/pydra/utils/hash.py +++ b/pydra/utils/hash.py @@ -105,13 +105,6 @@ def location_default(cls): def _location_default(self): return self.location_default() - @location.validator - def location_validator(self, _, location): - if not os.path.isdir(location): - raise ValueError( - f"Persistent cache location '{location}' is not a directory" - ) - @cleanup_period.default def cleanup_period_default(self): return int(os.environ.get(self.CLEANUP_ENV_VAR, 30)) diff --git a/pydra/utils/tests/test_hash.py b/pydra/utils/tests/test_hash.py index 56a7d9e68..68edaa4be 100644 --- a/pydra/utils/tests/test_hash.py +++ b/pydra/utils/tests/test_hash.py @@ -384,5 +384,5 @@ def test_persistent_hash_cache_not_dir(text_file): """ Test that an error is raised if the provided cache path is not a directory """ - with pytest.raises(ValueError, match="is not a directory"): + with pytest.raises(FileExistsError): PersistentCache(text_file.fspath) From db42246763537d6c38824fba844cf1d1f39493f0 Mon Sep 17 00:00:00 2001 From: Tom Close Date: Mon, 18 Mar 2024 09:43:45 +1100 Subject: [PATCH 3/3] changed exception to be ValueError when cache directory is a file, instead of a FileExistsError --- pydra/utils/hash.py | 7 ++++++- pydra/utils/tests/test_hash.py | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/pydra/utils/hash.py b/pydra/utils/hash.py index 4d8042cb3..4d3c6a74b 100644 --- a/pydra/utils/hash.py +++ b/pydra/utils/hash.py @@ -60,7 +60,12 @@ def location_converter(path: ty.Union[Path, str, None]) -> Path: if path is None: path = PersistentCache.location_default() path = Path(path) - path.mkdir(parents=True, exist_ok=True) + try: + path.mkdir(parents=True, exist_ok=True) + except FileExistsError: + raise ValueError( + f"provided path to persistent cache {path} is a file not a directory" + ) from None return path diff --git a/pydra/utils/tests/test_hash.py b/pydra/utils/tests/test_hash.py index 68edaa4be..e63e1c68b 100644 --- a/pydra/utils/tests/test_hash.py +++ b/pydra/utils/tests/test_hash.py @@ -384,5 +384,5 @@ def test_persistent_hash_cache_not_dir(text_file): """ Test that an error is raised if the provided cache path is not a directory """ - with pytest.raises(FileExistsError): + with pytest.raises(ValueError, match="not a directory"): PersistentCache(text_file.fspath)