From 170b2fadf5f0f84c86b1bdee2aa0f9cbba6ddaca Mon Sep 17 00:00:00 2001 From: martinbrose <13284268+martinbrose@users.noreply.github.com> Date: Sat, 23 Sep 2023 16:27:18 +0100 Subject: [PATCH 1/4] Add realpath to disk_usage to allow path-like obj --- src/huggingface_hub/file_download.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/huggingface_hub/file_download.py b/src/huggingface_hub/file_download.py index 9191f2d867..772baa6a66 100644 --- a/src/huggingface_hub/file_download.py +++ b/src/huggingface_hub/file_download.py @@ -973,7 +973,8 @@ def _check_disk_space(expected_size: int, target_dir: Union[str, Path]) -> None: """ target_dir = str(target_dir) - target_dir_free = shutil.disk_usage(target_dir).free + + target_dir_free = shutil.disk_usage(os.path.realpath(target_dir)).free has_enough_space = target_dir_free >= expected_size From c0b9b508effcc5a7a375010386f2facf09f83b2f Mon Sep 17 00:00:00 2001 From: martinbrose <13284268+martinbrose@users.noreply.github.com> Date: Mon, 25 Sep 2023 21:42:03 +0100 Subject: [PATCH 2/4] Add logic to address non-existent paths --- src/huggingface_hub/file_download.py | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/huggingface_hub/file_download.py b/src/huggingface_hub/file_download.py index 772baa6a66..ca2e8ebf19 100644 --- a/src/huggingface_hub/file_download.py +++ b/src/huggingface_hub/file_download.py @@ -972,18 +972,19 @@ def _check_disk_space(expected_size: int, target_dir: Union[str, Path]) -> None: The directory where the file will be stored after downloading. """ - target_dir = str(target_dir) - - target_dir_free = shutil.disk_usage(os.path.realpath(target_dir)).free - - has_enough_space = target_dir_free >= expected_size - - if not has_enough_space: - warnings.warn( - "Not enough free disk space to download the file. " - f"The expected file size is: {expected_size / 1e6:.2f} MB. " - f"The target location {target_dir} only has {target_dir_free / 1e6:.2f} MB free disk space." - ) + target_dir = Path(target_dir) # format as `Path` + for path in [target_dir] + list(target_dir.parents): # first check target_dir, then each parents one by one + try: + target_dir_free = shutil.disk_usage(path).free + if target_dir_free < expected_size: + warnings.warn( + "Not enough free disk space to download the file. " + f"The expected file size is: {expected_size / 1e6:.2f} MB. " + f"The target location {target_dir} only has {target_dir_free / 1e6:.2f} MB free disk space." + ) + return + except OSError: # raise on anything: file does not exist or space disk cannot be checked + pass @validate_hf_hub_args From 75a0c6ec4888b51a458245ec123231caa331e5dd Mon Sep 17 00:00:00 2001 From: martinbrose <13284268+martinbrose@users.noreply.github.com> Date: Mon, 25 Sep 2023 21:42:25 +0100 Subject: [PATCH 3/4] Add test cased for non-existent paths --- tests/test_file_download.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/test_file_download.py b/tests/test_file_download.py index cf841a7cd2..66f8e3cec9 100644 --- a/tests/test_file_download.py +++ b/tests/test_file_download.py @@ -112,6 +112,21 @@ def test_disk_usage_warning(self, disk_usage_mock: Mock) -> None: _check_disk_space(expected_size=self.expected_size, target_dir=disk_usage_mock) assert len(w) == 0 + def test_disk_usage_warning_with_non_existent_path(self) -> None: + # Test for not existent path + with warnings.catch_warnings(record=True) as w: + # Cause all warnings to always be triggered. + warnings.simplefilter("always") + _check_disk_space(expected_size=self.expected_size, target_dir="not_existent_path") + assert len(w) == 0 + + # Test for relative path + with warnings.catch_warnings(record=True) as w: + # Cause all warnings to always be triggered. + warnings.simplefilter("always") + _check_disk_space(expected_size=self.expected_size, target_dir="./not_existent_path") + assert len(w) == 0 + @with_production_testing class CachedDownloadTests(unittest.TestCase): From 65139817c537d4ea8e0ecb6999f1d5eb16471209 Mon Sep 17 00:00:00 2001 From: Lucain Date: Tue, 26 Sep 2023 09:58:31 +0200 Subject: [PATCH 4/4] Update tests/test_file_download.py --- tests/test_file_download.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_file_download.py b/tests/test_file_download.py index 66f8e3cec9..8c90e07e87 100644 --- a/tests/test_file_download.py +++ b/tests/test_file_download.py @@ -113,18 +113,18 @@ def test_disk_usage_warning(self, disk_usage_mock: Mock) -> None: assert len(w) == 0 def test_disk_usage_warning_with_non_existent_path(self) -> None: - # Test for not existent path + # Test for not existent (absolute) path with warnings.catch_warnings(record=True) as w: # Cause all warnings to always be triggered. warnings.simplefilter("always") - _check_disk_space(expected_size=self.expected_size, target_dir="not_existent_path") + _check_disk_space(expected_size=self.expected_size, target_dir="path/to/not_existent_path") assert len(w) == 0 - # Test for relative path + # Test for not existent (relative) path with warnings.catch_warnings(record=True) as w: # Cause all warnings to always be triggered. warnings.simplefilter("always") - _check_disk_space(expected_size=self.expected_size, target_dir="./not_existent_path") + _check_disk_space(expected_size=self.expected_size, target_dir="/path/to/not_existent_path") assert len(w) == 0