Repository: huggingface/huggingface_hub
Pull request #203 (2021-07-16 12:07:54)

No need for token to understand HF urls
This patches an issue where the token was necessary to understand links to the Hugging Face Hub, which was involuntary behavior.

Fixes https://github.com/huggingface/huggingface_hub/issues/186

LysandreJik: This PR removes the ability to clone from a repository using only its model ID, guessing its namespace in return. This is unfortunately due to the models that live at the root (`bert-base-cased`, `gpt2`).

Patch:
diff --git a/src/huggingface_hub/hf_api.py b/src/huggingface_hub/hf_api.py
index 6e2c05d3..229728f7 100644
--- a/src/huggingface_hub/hf_api.py
+++ b/src/huggingface_hub/hf_api.py
@@ -58,6 +58,8 @@ def repo_type_and_id_from_hf_id(hf_id: str):
 
     if is_hf_url:
         namespace, repo_id = url_segments[-2:]
+        if namespace == "huggingface.co":
+            namespace = None
         if len(url_segments) > 2 and "huggingface.co" not in url_segments[-3]:
             repo_type = url_segments[-3]
         else:
diff --git a/src/huggingface_hub/repository.py b/src/huggingface_hub/repository.py
index dda17c3e..13ee9e66 100644
--- a/src/huggingface_hub/repository.py
+++ b/src/huggingface_hub/repository.py
@@ -120,7 +120,7 @@ class Repository:
         local_dir: str,
         clone_from: Optional[str] = None,
         repo_type: Optional[str] = None,
-        use_auth_token: Union[bool, str, None] = None,
+        use_auth_token: Union[bool, str] = True,
         git_user: Optional[str] = None,
         git_email: Optional[str] = None,
     ):
@@ -223,24 +223,24 @@ class Repository:
         token = use_auth_token if use_auth_token is not None else self.huggingface_token
         api = HfApi()
 
-        if token is not None:
-            whoami_info = api.whoami(token)
-            user = whoami_info["name"]
-            valid_organisations = [org["name"] for org in whoami_info["orgs"]]
-            repo_type, namespace, repo_id = repo_type_and_id_from_hf_id(repo_url)
+        repo_type, namespace, repo_id = repo_type_and_id_from_hf_id(repo_url)
 
-            if namespace is None:
-                namespace = user
+        if repo_type is not None:
+            self.repo_type = repo_type
 
-            if repo_type is not None:
-                self.repo_type = repo_type
+        repo_url = ENDPOINT + "/"
 
-            repo_url = ENDPOINT + "/"
+        if self.repo_type in REPO_TYPES_URL_PREFIXES:
+            repo_url += REPO_TYPES_URL_PREFIXES[self.repo_type]
 
-            if self.repo_type in REPO_TYPES_URL_PREFIXES:
-                repo_url += REPO_TYPES_URL_PREFIXES[self.repo_type]
+        if token is not None:
+            whoami_info = api.whoami(token)
+            user = whoami_info["name"]
+            valid_organisations = [org["name"] for org in whoami_info["orgs"]]
 
-            repo_url += f"{namespace}/{repo_id}"
+            if namespace is not None:
+                repo_url += f"{namespace}/"
+            repo_url += repo_id
 
             repo_url = repo_url.replace("https://", f"https://user:{token}@")
 
@@ -252,6 +252,11 @@ class Repository:
                     organization=namespace,
                     exist_ok=True,
                 )
+        else:
+            if namespace is not None:
+                repo_url += f"{namespace}/"
+            repo_url += repo_id
+
         # For error messages, it's cleaner to show the repo url without the token.
         clean_repo_url = re.sub(r"https://.*@", "https://", repo_url)
         try:
@@ -265,7 +270,7 @@ class Repository:
 
             # checks if repository is initialized in a empty repository or in one with files
             if len(os.listdir(self.local_dir)) == 0:
-                logger.debug(f"Cloning {clean_repo_url} into local empty directory.")
+                logger.warning(f"Cloning {clean_repo_url} into local empty directory.")
                 subprocess.run(
                     ["git", "clone", repo_url, "."],
                     stderr=subprocess.PIPE,


Test patch:
diff --git a/tests/test_hf_api.py b/tests/test_hf_api.py
index 6a7356d4..ffb941c6 100644
--- a/tests/test_hf_api.py
+++ b/tests/test_hf_api.py
@@ -561,6 +561,7 @@ class HfLargefilesTest(HfApiCommonTest):
 class HfApiMiscTest(unittest.TestCase):
     def test_repo_type_and_id_from_hf_id(self):
         possible_values = {
+            "https://huggingface.co/id": [None, None, "id"],
             "https://huggingface.co/user/id": [None, "user", "id"],
             "https://huggingface.co/datasets/user/id": ["dataset", "user", "id"],
             "https://huggingface.co/spaces/user/id": ["space", "user", "id"],
diff --git a/tests/test_repository.py b/tests/test_repository.py
index 587faae9..f3403d28 100644
--- a/tests/test_repository.py
+++ b/tests/test_repository.py
@@ -26,7 +26,7 @@ from huggingface_hub.hf_api import HfApi
 from huggingface_hub.repository import Repository, is_tracked_with_lfs
 
 from .testing_constants import ENDPOINT_STAGING, PASS, USER
-from .testing_utils import set_write_permission_and_retry
+from .testing_utils import set_write_permission_and_retry, with_production_testing
 
 
 REPO_NAME = "repo-{}".format(int(time.time() * 10e3))
@@ -312,35 +312,65 @@ class RepositoryTest(RepositoryCommonTest):
         self.assertTrue("model.bin" in files)
 
     def test_clone_with_repo_name_and_no_namespace(self):
-        clone = Repository(
-            REPO_NAME,
+        self.assertRaises(
+            OSError,
+            Repository,
+            f"{WORKING_REPO_DIR}/{REPO_NAME}",
             clone_from=REPO_NAME,
             use_auth_token=self._token,
             git_user="ci",
             git_email="ci@dummy.com",
         )
 
-        with clone.commit("Commit"):
-            # Create dummy files
-            # one is lfs-tracked, the other is not.
-            with open("dummy.txt", "w") as f:
-                f.write("hello")
-            with open("model.bin", "w") as f:
-                f.write("hello")
+    def test_clone_with_repo_name_user_and_no_auth_token(self):
+        # Create repo
+        Repository(
+            f"{WORKING_REPO_DIR}/{REPO_NAME}",
+            clone_from=f"{USER}/{REPO_NAME}",
+            git_user="ci",
+            git_email="ci@dummy.com",
+        )
 
-        shutil.rmtree(REPO_NAME)
+        # Instantiate it without token
+        Repository(
+            f"{WORKING_REPO_DIR}/{REPO_NAME}",
+            clone_from=f"{USER}/{REPO_NAME}",
+            git_user="ci",
+            git_email="ci@dummy.com",
+        )
 
+    def test_clone_with_repo_name_org_and_no_auth_token(self):
+        # Create repo
         Repository(
             f"{WORKING_REPO_DIR}/{REPO_NAME}",
-            clone_from=REPO_NAME,
             use_auth_token=self._token,
+            clone_from=f"valid_org/{REPO_NAME}",
             git_user="ci",
             git_email="ci@dummy.com",
         )
 
-        files = os.listdir(f"{WORKING_REPO_DIR}/{REPO_NAME}")
-        self.assertTrue("dummy.txt" in files)
-        self.assertTrue("model.bin" in files)
+        # Instantiate it without token
+        Repository(
+            f"{WORKING_REPO_DIR}/{REPO_NAME}",
+            clone_from=f"valid_org/{REPO_NAME}",
+            git_user="ci",
+            git_email="ci@dummy.com",
+        )
+
+    @with_production_testing
+    def test_clone_repo_at_root(self):
+        os.environ["GIT_LFS_SKIP_SMUDGE"] = "1"
+        Repository(
+            f"{WORKING_REPO_DIR}/{REPO_NAME}",
+            clone_from="bert-base-cased",
+        )
+
+        shutil.rmtree(f"{WORKING_REPO_DIR}/{REPO_NAME}")
+
+        Repository(
+            f"{WORKING_REPO_DIR}/{REPO_NAME}",
+            clone_from="https://huggingface.co/bert-base-cased",
+        )
 
 
 class RepositoryAutoLFSTrackingTest(RepositoryCommonTest):
@@ -622,12 +652,15 @@ class RepositoryDatasetTest(RepositoryCommonTest):
                 token=self._token, name=REPO_NAME, repo_type="dataset"
             )
         except requests.exceptions.HTTPError:
-            self._api.delete_repo(
-                token=self._token,
-                organization="valid_org",
-                name=REPO_NAME,
-                repo_type="dataset",
-            )
+            try:
+                self._api.delete_repo(
+                    token=self._token,
+                    organization="valid_org",
+                    name=REPO_NAME,
+                    repo_type="dataset",
+                )
+            except requests.exceptions.HTTPError:
+                pass
 
         shutil.rmtree(
             f"{WORKING_DATASET_DIR}/{REPO_NAME}", onerror=set_write_permission_and_retry
@@ -721,7 +754,9 @@ class RepositoryDatasetTest(RepositoryCommonTest):
         self.assertTrue("test.py" in files)
 
     def test_clone_with_repo_name_and_no_namespace(self):
-        clone = Repository(
+        self.assertRaises(
+            OSError,
+            Repository,
             f"{WORKING_DATASET_DIR}/{REPO_NAME}",
             clone_from=REPO_NAME,
             repo_type="dataset",
@@ -730,21 +765,42 @@ class RepositoryDatasetTest(RepositoryCommonTest):
             git_email="ci@dummy.com",
         )
 
-        with clone.commit("Commit"):
-            for file in os.listdir(DATASET_FIXTURE):
-                shutil.copyfile(pathlib.Path(DATASET_FIXTURE) / file, file)
+    def test_clone_with_repo_name_user_and_no_auth_token(self):
+        # Create repo
+        Repository(
+            f"{WORKING_DATASET_DIR}/{REPO_NAME}",
+            clone_from=f"{USER}/{REPO_NAME}",
+            repo_type="dataset",
+            use_auth_token=self._token,
+            git_user="ci",
+            git_email="ci@dummy.com",
+        )
 
-        shutil.rmtree(f"{WORKING_DATASET_DIR}/{REPO_NAME}")
+        # Instantiate it without token
+        Repository(
+            f"{WORKING_DATASET_DIR}/{REPO_NAME}",
+            clone_from=f"{USER}/{REPO_NAME}",
+            repo_type="dataset",
+            git_user="ci",
+            git_email="ci@dummy.com",
+        )
 
+    def test_clone_with_repo_name_org_and_no_auth_token(self):
+        # Create repo
         Repository(
             f"{WORKING_DATASET_DIR}/{REPO_NAME}",
-            clone_from=REPO_NAME,
-            use_auth_token=self._token,
+            clone_from=f"valid_org/{REPO_NAME}",
             repo_type="dataset",
+            use_auth_token=self._token,
             git_user="ci",
             git_email="ci@dummy.com",
         )
 
-        files = os.listdir(f"{WORKING_DATASET_DIR}/{REPO_NAME}")
-        self.assertTrue("some_text.txt" in files)
-        self.assertTrue("test.py" in files)
+        # Instantiate it without token
+        Repository(
+            f"{WORKING_DATASET_DIR}/{REPO_NAME}",
+            clone_from=f"valid_org/{REPO_NAME}",
+            repo_type="dataset",
+            git_user="ci",
+            git_email="ci@dummy.com",
+        )
diff --git a/tests/testing_utils.py b/tests/testing_utils.py
index e7b02ed6..7bacd9d7 100644
--- a/tests/testing_utils.py
+++ b/tests/testing_utils.py
@@ -164,4 +164,9 @@ def with_production_testing(func):
         ENDPOINT_PRODUCTION,
     )
 
-    return hf_api(file_download(func))
+    repository = patch(
+        "huggingface_hub.repository.ENDPOINT",
+        ENDPOINT_PRODUCTION,
+    )
+
+    return repository(hf_api(file_download(func)))