Repository: huggingface/huggingface_hub
Pull request #2059 (2024-02-28 20:48:12)

Enable `python-xdist` on all tests
Enable `python-xdist` on all tests. This reduces overall test execution by 55% on CI down to a around 6min (from 12min).
After experimenting, using 4 instead of `auto` processes seems to avoid too much context trashing. 

|                 | Total duration |  | Comment                    |  
|----------|---------------|-|----------------------|
| Baseline        | 12m 13s        | [Workflow](https://github.com/huggingface/huggingface_hub/actions/runs/8083119316)         |                                           |
| xdist `-n auto` | 6m 33s         | [Workflow](https://github.com/huggingface/huggingface_hub/actions/runs/8086882581?pr=2059) | ubuntu a bit slower, windows a lot faster | 
| xdist `-n 4`    | 5m 46s            |  [Workflow](https://github.com/huggingface/huggingface_hub/actions/runs/8087001450) |

Open questions:
- [ ] The combinations that do work, can they be folded into a single test matrix configurarion (essentially whether there is still a need for Repository only/Everything else/torch; the others like lfs/fastai/tensorflow are still required as they need specific python versions. Even though this can be improved, it's likely not worth it as the windows configuration will be the dominating factor for overall CI time. 
- [x] Do Windows tests pass at all through xdist?
- [x] Should the `"Repository only"` tests keep using `-n 4` vs `-n auto` (rate limit concerns)

HuggingFaceDocBuilderDev: The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/huggingface_hub/pr_2059). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/huggingface/huggingface_hub/pull/2059?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=huggingface) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 80.67%. Comparing base [(`e7f243c`)](https://app.codecov.io/gh/huggingface/huggingface_hub/commit/e7f243c5fea94237cdca84b71f0d5958c40ed38a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=huggingface) to head [(`8cb7130`)](https://app.codecov.io/gh/huggingface/huggingface_hub/pull/2059?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=huggingface).


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


```diff
@@            Coverage Diff             @@
##             main    #2059      +/-   ##
==========================================
- Coverage   80.70%   80.67%   -0.03%     
==========================================
  Files          71       71              
  Lines        8519     8519              
==========================================
- Hits         6875     6873       -2     
- Misses       1644     1646       +2     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/huggingface/huggingface_hub/pull/2059?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/hub_mixin.py b/src/huggingface_hub/hub_mixin.py
index 6d864180..013b7a47 100644
--- a/src/huggingface_hub/hub_mixin.py
+++ b/src/huggingface_hub/hub_mixin.py
@@ -270,6 +270,9 @@ class ModelHubMixin:
 
                 # Forward config to model initialization
                 model_kwargs["config"] = config
+            elif any(param.kind == inspect.Parameter.VAR_KEYWORD for param in init_parameters.values()):
+                # If __init__ accepts **kwargs, let's forward the config as well (as a dict)
+                model_kwargs["config"] = config
 
         instance = cls._from_pretrained(
             model_id=str(model_id),
diff --git a/src/huggingface_hub/utils/_errors.py b/src/huggingface_hub/utils/_errors.py
index 9cef65bb..d6482cc8 100644
--- a/src/huggingface_hub/utils/_errors.py
+++ b/src/huggingface_hub/utils/_errors.py
@@ -357,6 +357,15 @@ def hf_raise_for_status(response: Response, endpoint_name: Optional[str] = None)
             )
             raise BadRequestError(message, response=response) from e
 
+        elif response.status_code == 403:
+            message = (
+                f"\n\n{response.status_code} Forbidden: {error_message}."
+                + f"\nCannot access content at: {response.url}."
+                + "\nIf you are trying to create or update content,"
+                + "make sure you have a token with the `write` role."
+            )
+            raise HfHubHTTPError(message, response=response) from e
+
         # Convert `HTTPError` into a `HfHubHTTPError` to display request information
         # as well (request id and/or server error message)
         raise HfHubHTTPError(str(e), response=response) from e


Test patch:
diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml
index c4d9375e..280b98c8 100644
--- a/.github/workflows/python-tests.yml
+++ b/.github/workflows/python-tests.yml
@@ -91,13 +91,13 @@ jobs:
 
             "Repository only")
               # Run repo tests concurrently
-              PYTEST="$PYTEST ../tests -k 'TestRepository' -n4"
+              PYTEST="$PYTEST ../tests -k 'TestRepository' -n 4"
               echo $PYTEST
               eval $PYTEST
             ;;
 
             "Everything else")
-              PYTEST="$PYTEST ../tests -k 'not TestRepository'"
+              PYTEST="$PYTEST ../tests -k 'not TestRepository' -n 4"
               echo $PYTEST
               eval $PYTEST
             ;;
@@ -156,7 +156,6 @@ jobs:
         run: |
           pip install --upgrade pip
           pip install .[testing]
-        # sudo apt install -y libsndfile1-dev
 
       # Easy debugging if CI is broken
       - name: Pip freeze
@@ -165,7 +164,7 @@ jobs:
       # Run tests
       - name: Run tests
         working-directory: ./src # For code coverage to work
-        run: python -m pytest --cov=./huggingface_hub --cov-report=xml:../coverage.xml --vcr-record=none --reruns 5 --reruns-delay 1 --only-rerun '(OSError|Timeout|HTTPError.*502|HTTPError.*504|not less than or equal to 0.01)' ../tests
+        run: python -m pytest -n 4 --cov=./huggingface_hub --cov-report=xml:../coverage.xml --vcr-record=none --reruns 5 --reruns-delay 1 --only-rerun '(OSError|Timeout|HTTPError.*502|HTTPError.*504|not less than or equal to 0.01)' ../tests
 
       # Upload code coverage
       - name: Upload coverage reports to Codecov with GitHub Action
diff --git a/tests/test_hub_mixin.py b/tests/test_hub_mixin.py
index 88d70dcb..4ced2118 100644
--- a/tests/test_hub_mixin.py
+++ b/tests/test_hub_mixin.py
@@ -38,8 +38,11 @@ class BaseModel:
         **kwargs,
     ) -> "BaseModel":
         # Little hack but in practice NO-ONE is creating 5 inherited classes for their framework :D
-        if inspect.signature(cls.__init__).parameters.get("config"):
+        init_parameters = inspect.signature(cls.__init__).parameters
+        if init_parameters.get("config"):
             return cls(config=kwargs.get("config"))
+        if init_parameters.get("kwargs"):
+            return cls(**kwargs)
         return cls()
 
 
@@ -68,6 +71,11 @@ class DummyModelConfigAsOptionalDict(BaseModel, ModelHubMixin):
         pass
 
 
+class DummyModelWithKwargs(BaseModel, ModelHubMixin):
+    def __init__(self, **kwargs):
+        pass
+
+
 @pytest.mark.usefixtures("fx_cache_dir")
 class HubMixinTest(unittest.TestCase):
     cache_dir: Path
@@ -132,6 +140,34 @@ class HubMixinTest(unittest.TestCase):
         model.save_pretrained(self.cache_dir, config=CONFIG_AS_DICT)
         self.assert_valid_config_json()
 
+    def test_init_accepts_kwargs_no_config(self):
+        """
+        Test that if `__init__` accepts **kwargs and config file doesn't exist then no 'config' kwargs is passed.
+
+        Regression test. See https://github.com/huggingface/huggingface_hub/pull/2058.
+        """
+        model = DummyModelWithKwargs()
+        model.save_pretrained(self.cache_dir)
+        with patch.object(
+            DummyModelWithKwargs, "_from_pretrained", return_value=DummyModelWithKwargs()
+        ) as from_pretrained_mock:
+            model = DummyModelWithKwargs.from_pretrained(self.cache_dir)
+            assert "config" not in from_pretrained_mock.call_args_list[0].kwargs
+
+    def test_init_accepts_kwargs_with_config(self):
+        """
+        Test that if `__init__` accepts **kwargs and config file exists then the 'config' kwargs is passed.
+
+        Regression test. See https://github.com/huggingface/huggingface_hub/pull/2058.
+        """
+        model = DummyModelWithKwargs()
+        model.save_pretrained(self.cache_dir, config=CONFIG_AS_DICT)
+        with patch.object(
+            DummyModelWithKwargs, "_from_pretrained", return_value=DummyModelWithKwargs()
+        ) as from_pretrained_mock:
+            model = DummyModelWithKwargs.from_pretrained(self.cache_dir)
+            assert "config" in from_pretrained_mock.call_args_list[0].kwargs
+
     def test_save_pretrained_with_push_to_hub(self):
         repo_id = repo_name("save")
         save_directory = self.cache_dir / repo_id
diff --git a/tests/test_utils_errors.py b/tests/test_utils_errors.py
index 2b60e7d6..2ecc018d 100644
--- a/tests/test_utils_errors.py
+++ b/tests/test_utils_errors.py
@@ -49,6 +49,19 @@ class TestErrorUtils(unittest.TestCase):
         self.assertEqual(context.exception.response.status_code, 401)
         self.assertIn("Request ID: 123", str(context.exception))
 
+    def test_hf_raise_for_status_403_wrong_token_scope(self) -> None:
+        response = Response()
+        response.headers = {"X-Request-Id": 123, "X-Error-Message": "specific error message"}
+        response.status_code = 403
+        response.request = PreparedRequest()
+        response.request.url = "https://huggingface.co/api/repos/create"
+        expected_message_part = "403 Forbidden: specific error message"
+        with self.assertRaisesRegex(HfHubHTTPError, expected_message_part) as context:
+            hf_raise_for_status(response)
+
+        self.assertEqual(context.exception.response.status_code, 403)
+        self.assertIn("Request ID: 123", str(context.exception))
+
     def test_hf_raise_for_status_401_not_repo_url(self) -> None:
         response = Response()
         response.headers = {"X-Request-Id": 123}