Repository: huggingface/huggingface_hub
Pull request #1728 (2023-10-12 16:20:37)

Fix a bug to respect the HF_HUB_ETAG_TIMEOUT.
The purpose of this pr is to fix a bug that seems the HF_HUB_ETAG_TIMEOUT is not respected.
It looks like the HF_HUB_DOWNLOAD_TIMEOUT is respected in places that the HF_HUB_ETAG_TIMEOUT should be respected.

Shahafgo: @Wauplin , a small fix for the previous pr, seems like when I took your suggestion, there were couple of places you put HF_HUB_DOWNLOAD_TIMEOUT instead of HF_HUB_ETAG_TIMEOUT.
So it seems that on the main branch for now, only the HF_HUB_DOWNLOAD_TIMEOUT is respected (: 
HuggingFaceDocBuilderDev: The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/huggingface_hub/pr_1728). All of your documentation changes will be reflected on that endpoint.
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/huggingface/huggingface_hub/pull/1728?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=huggingface) Report
Attention: `2 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`30f5ed4`)](https://app.codecov.io/gh/huggingface/huggingface_hub/commit/30f5ed4f6fd6c7337338d008bc6c2285240072a4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=huggingface) 49.23% compared to head [(`9b1a2b4`)](https://app.codecov.io/gh/huggingface/huggingface_hub/pull/1728?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=huggingface) 82.04%.

> :exclamation: Current head 9b1a2b4 differs from pull request most recent head 357e0f9. Consider uploading reports for the commit 357e0f9 to get more accurate results

<details><summary>Additional details and impacted files</summary>


```diff
@@             Coverage Diff             @@
##             main    #1728       +/-   ##
===========================================
+ Coverage   49.23%   82.04%   +32.81%     
===========================================
  Files          62       62               
  Lines        7278     7278               
===========================================
+ Hits         3583     5971     +2388     
+ Misses       3695     1307     -2388     
```


| [Files](https://app.codecov.io/gh/huggingface/huggingface_hub/pull/1728?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=huggingface) | Coverage Δ | |
|---|---|---|
| [src/huggingface\_hub/file\_download.py](https://app.codecov.io/gh/huggingface/huggingface_hub/pull/1728?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=huggingface#diff-c3JjL2h1Z2dpbmdmYWNlX2h1Yi9maWxlX2Rvd25sb2FkLnB5) | `84.97% <50.00%> (+35.99%)` | :arrow_up: |

... and [43 files with indirect coverage changes](https://app.codecov.io/gh/huggingface/huggingface_hub/pull/1728/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=huggingface)


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/huggingface/huggingface_hub/pull/1728?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=huggingface).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=huggingface).


Patch:
diff --git a/src/huggingface_hub/file_download.py b/src/huggingface_hub/file_download.py
index d7f2bf00..7f540eea 100644
--- a/src/huggingface_hub/file_download.py
+++ b/src/huggingface_hub/file_download.py
@@ -33,6 +33,7 @@ from .constants import (
     HF_HUB_DISABLE_SYMLINKS_WARNING,
     HF_HUB_DOWNLOAD_TIMEOUT,
     HF_HUB_ENABLE_HF_TRANSFER,
+    HF_HUB_ETAG_TIMEOUT,
     HUGGINGFACE_CO_URL_TEMPLATE,
     HUGGINGFACE_HEADER_X_LINKED_ETAG,
     HUGGINGFACE_HEADER_X_LINKED_SIZE,
@@ -664,9 +665,9 @@ def cached_download(
 
     </Tip>
     """
-    if HF_HUB_DOWNLOAD_TIMEOUT != DEFAULT_DOWNLOAD_TIMEOUT:
+    if HF_HUB_ETAG_TIMEOUT != DEFAULT_ETAG_TIMEOUT:
         # Respect environment variable above user value
-        etag_timeout = HF_HUB_DOWNLOAD_TIMEOUT
+        etag_timeout = HF_HUB_ETAG_TIMEOUT
 
     if not legacy_cache_layout:
         warnings.warn(
@@ -1150,9 +1151,9 @@ def hf_hub_download(
 
     </Tip>
     """
-    if HF_HUB_DOWNLOAD_TIMEOUT != DEFAULT_DOWNLOAD_TIMEOUT:
+    if HF_HUB_ETAG_TIMEOUT != DEFAULT_ETAG_TIMEOUT:
         # Respect environment variable above user value
-        etag_timeout = HF_HUB_DOWNLOAD_TIMEOUT
+        etag_timeout = HF_HUB_ETAG_TIMEOUT
 
     if force_filename is not None:
         warnings.warn(


Test patch:
diff --git a/tests/test_file_download.py b/tests/test_file_download.py
index 8c90e07e..63de900c 100644
--- a/tests/test_file_download.py
+++ b/tests/test_file_download.py
@@ -1025,6 +1025,65 @@ class TestNormalizeEtag(unittest.TestCase):
         return _normalize_etag(response.headers.get(HUGGINGFACE_HEADER_X_LINKED_ETAG) or response.headers.get("ETag"))
 
 
+@with_production_testing
+class TestEtagTimeoutConfig(unittest.TestCase):
+    @patch("huggingface_hub.file_download.DEFAULT_ETAG_TIMEOUT", 10)
+    @patch("huggingface_hub.file_download.HF_HUB_ETAG_TIMEOUT", 10)
+    def test_etag_timeout_default_value(self):
+        with SoftTemporaryDirectory() as cache_dir:
+            with patch.object(
+                huggingface_hub.file_download,
+                "get_hf_file_metadata",
+                wraps=huggingface_hub.file_download.get_hf_file_metadata,
+            ) as mock_etag_call:
+                hf_hub_download(DUMMY_MODEL_ID, filename=CONFIG_NAME, cache_dir=cache_dir)
+                kwargs = mock_etag_call.call_args.kwargs
+                self.assertIn("timeout", kwargs)
+                self.assertEqual(kwargs["timeout"], 10)
+
+    @patch("huggingface_hub.file_download.DEFAULT_ETAG_TIMEOUT", 10)
+    @patch("huggingface_hub.file_download.HF_HUB_ETAG_TIMEOUT", 10)
+    def test_etag_timeout_parameter_value(self):
+        with SoftTemporaryDirectory() as cache_dir:
+            with patch.object(
+                huggingface_hub.file_download,
+                "get_hf_file_metadata",
+                wraps=huggingface_hub.file_download.get_hf_file_metadata,
+            ) as mock_etag_call:
+                hf_hub_download(DUMMY_MODEL_ID, filename=CONFIG_NAME, cache_dir=cache_dir, etag_timeout=12)
+                kwargs = mock_etag_call.call_args.kwargs
+                self.assertIn("timeout", kwargs)
+                self.assertEqual(kwargs["timeout"], 12)  # passed as parameter, takes priority
+
+    @patch("huggingface_hub.file_download.DEFAULT_ETAG_TIMEOUT", 10)
+    @patch("huggingface_hub.file_download.HF_HUB_ETAG_TIMEOUT", 15)  # takes priority
+    def test_etag_timeout_set_as_env_variable(self):
+        with SoftTemporaryDirectory() as cache_dir:
+            with patch.object(
+                huggingface_hub.file_download,
+                "get_hf_file_metadata",
+                wraps=huggingface_hub.file_download.get_hf_file_metadata,
+            ) as mock_etag_call:
+                hf_hub_download(DUMMY_MODEL_ID, filename=CONFIG_NAME, cache_dir=cache_dir)
+                kwargs = mock_etag_call.call_args.kwargs
+                self.assertIn("timeout", kwargs)
+                self.assertEqual(kwargs["timeout"], 15)
+
+    @patch("huggingface_hub.file_download.DEFAULT_ETAG_TIMEOUT", 10)
+    @patch("huggingface_hub.file_download.HF_HUB_ETAG_TIMEOUT", 15)  # takes priority
+    def test_etag_timeout_set_as_env_variable_parameter_ignored(self):
+        with SoftTemporaryDirectory() as cache_dir:
+            with patch.object(
+                huggingface_hub.file_download,
+                "get_hf_file_metadata",
+                wraps=huggingface_hub.file_download.get_hf_file_metadata,
+            ) as mock_etag_call:
+                hf_hub_download(DUMMY_MODEL_ID, filename=CONFIG_NAME, cache_dir=cache_dir, etag_timeout=12)
+                kwargs = mock_etag_call.call_args.kwargs
+                self.assertIn("timeout", kwargs)
+                self.assertEqual(kwargs["timeout"], 15)  # passed value ignored, HF_HUB_ETAG_TIMEOUT takes priority
+
+
 def _recursive_chmod(path: str, mode: int) -> None:
     # Taken from https://stackoverflow.com/a/2853934
     for root, dirs, files in os.walk(path):