Skip to content

Commit

Permalink
🗑 Deprecate token in read-only methods of HfApi in favor of `use_…
Browse files Browse the repository at this point in the history
…auth_token` (#928)

* 🚚 Proposal: `use_auth_token` for read only `HfApi` methods

This PR deprecates the `token` argument in read-only methods of `HfApi`
in favor of `use_auth_token`.

`use_auth_token` is more flexible as it allows not passing a token,
whereas `token=None` fetches the token from `HfFolder`.

I personally prefer the semantics of `use_auth_token`, that I find very legible:
- `use_auth_token=False`
- `use_auth_token="token"`
- `use_auth_token=True`

cc @lhoestq

* 🔊 Deprecate in 0.12

* â™» factorization

* ✅ Update unit tests to ensure a warning is raised

* 🎨 Smol typing fix

* 💄 Code format

* 🩹
  • Loading branch information
SBrandeis authored Sep 13, 2022
1 parent b35f817 commit 55bb9ec
Show file tree
Hide file tree
Showing 9 changed files with 157 additions and 61 deletions.
2 changes: 1 addition & 1 deletion src/huggingface_hub/_snapshot_download.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ def snapshot_download(
# if we have internet connection we retrieve the correct folder name from the huggingface api
_api = HfApi()
repo_info = _api.repo_info(
repo_id=repo_id, repo_type=repo_type, revision=revision, token=token
repo_id=repo_id, repo_type=repo_type, revision=revision, use_auth_token=token
)
filtered_repo_files = list(
filter_repo_objects(
Expand Down
145 changes: 99 additions & 46 deletions src/huggingface_hub/hf_api.py

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/huggingface_hub/inference_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ def __init__(
self.headers["Authorization"] = f"Bearer {token}"

# Configure task
model_info = HfApi().model_info(repo_id=repo_id, token=token)
model_info = HfApi().model_info(repo_id=repo_id, use_auth_token=token)
if not model_info.pipeline_tag and not task:
raise ValueError(
"Task not specified in the repository. Please add it to the model card"
Expand Down
4 changes: 3 additions & 1 deletion src/huggingface_hub/keras_mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,9 @@ def push_to_hub_keras(
# Delete previous log files from Hub
operations += [
CommitOperationDelete(path_in_repo=file)
for file in api.list_repo_files(repo_id=repo_id, token=token)
for file in api.list_repo_files(
repo_id=repo_id, use_auth_token=token
)
if file.startswith("logs/")
]

Expand Down
4 changes: 3 additions & 1 deletion src/huggingface_hub/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,9 @@ def clone_from(self, repo_url: str, use_auth_token: Union[bool, str, None] = Non
if namespace == user or namespace in valid_organisations:
try:
_ = HfApi().repo_info(
f"{repo_id}", repo_type=self.repo_type, token=token
f"{repo_id}",
repo_type=self.repo_type,
use_auth_token=token,
)
except HTTPError:
if self.repo_type == "space":
Expand Down
35 changes: 32 additions & 3 deletions tests/test_hf_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -609,11 +609,13 @@ def test_delete_file(self):
self._api.delete_repo(repo_id=REPO_NAME, token=self._token)

def test_get_full_repo_name(self):
repo_name_with_no_org = self._api.get_full_repo_name("model", token=self._token)
repo_name_with_no_org = self._api.get_full_repo_name(
"model", use_auth_token=self._token
)
self.assertEqual(repo_name_with_no_org, f"{USER}/model")

repo_name_with_no_org = self._api.get_full_repo_name(
"model", organization="org", token=self._token
"model", organization="org", use_auth_token=self._token
)
self.assertEqual(repo_name_with_no_org, "org/model")

Expand Down Expand Up @@ -1468,11 +1470,38 @@ def test_model_info(self):
):
_ = self._api.model_info(repo_id=f"{USER}/{self.REPO_NAME}")
# Test we can access model info with a token
with self.assertWarns(FutureWarning):
model_info = self._api.model_info(
repo_id=f"{USER}/{self.REPO_NAME}", token=self._token
)
self.assertIsInstance(model_info, ModelInfo)
model_info = self._api.model_info(
repo_id=f"{USER}/{self.REPO_NAME}", token=self._token
repo_id=f"{USER}/{self.REPO_NAME}", use_auth_token=self._token
)
self.assertIsInstance(model_info, ModelInfo)

def test_dataset_info(self):
shutil.rmtree(os.path.dirname(HfFolder.path_token), ignore_errors=True)
# Test we cannot access model info without a token
with self.assertRaisesRegex(
requests.exceptions.HTTPError,
re.compile(
r"401 Client Error(.+)\(Request ID: .+\)(.*)Repository Not Found",
flags=re.DOTALL,
),
):
_ = self._api.dataset_info(repo_id=f"{USER}/{self.REPO_NAME}")
# Test we can access model info with a token
with self.assertWarns(FutureWarning):
dataset_info = self._api.dataset_info(
repo_id=f"{USER}/{self.REPO_NAME}", token=self._token
)
self.assertIsInstance(dataset_info, DatasetInfo)
dataset_info = self._api.dataset_info(
repo_id=f"{USER}/{self.REPO_NAME}", use_auth_token=self._token
)
self.assertIsInstance(dataset_info, DatasetInfo)

def test_list_private_datasets(self):
orig = len(self._api.list_datasets())
new = len(self._api.list_datasets(use_auth_token=self._token))
Expand Down
8 changes: 5 additions & 3 deletions tests/test_hubmixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ def test_push_to_hub_via_http_basic(self):
)

# Test model id exists
model_info = self._api.model_info(repo_id, token=self._token)
model_info = self._api.model_info(repo_id, use_auth_token=self._token)
self.assertEqual(model_info.modelId, repo_id)

# Test config has been pushed to hub
Expand All @@ -204,7 +204,7 @@ def test_push_to_hub_via_git_deprecated(self):
use_auth_token=self._token,
)

model_info = self._api.model_info(repo_id, token=self._token)
model_info = self._api.model_info(repo_id, use_auth_token=self._token)
self.assertEqual(model_info.modelId, repo_id)
self._api.delete_repo(repo_id=repo_id, token=self._token)

Expand Down Expand Up @@ -236,7 +236,9 @@ def test_push_to_hub_via_git_use_lfs_by_default(self):
git_email="ci@dummy.com",
)

model_info = self._api.model_info(f"{USER}/{REPO_NAME}", token=self._token)
model_info = self._api.model_info(
f"{USER}/{REPO_NAME}", use_auth_token=self._token
)

self.assertTrue("large_file.txt" in [f.rfilename for f in model_info.siblings])
self._api.delete_repo(repo_id=f"{REPO_NAME}", token=self._token)
16 changes: 12 additions & 4 deletions tests/test_inference_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,12 @@ def test_inference_with_dict_inputs(self):
@with_production_testing
def test_inference_with_audio(self):
api = InferenceApi("facebook/wav2vec2-base-960h")
dataset = datasets.load_dataset(
"patrickvonplaten/librispeech_asr_dummy", "clean", split="validation"
)
with self.assertWarns(FutureWarning):
dataset = datasets.load_dataset(
"patrickvonplaten/librispeech_asr_dummy",
"clean",
split="validation",
)
data = self.read(dataset["file"][0])
result = api(data=data)
self.assertIsInstance(result, dict)
Expand All @@ -78,7 +81,12 @@ def test_inference_with_audio(self):
@with_production_testing
def test_inference_with_image(self):
api = InferenceApi("google/vit-base-patch16-224")
dataset = datasets.load_dataset("Narsil/image_dummy", "image", split="test")
with self.assertWarns(FutureWarning):
dataset = datasets.load_dataset(
"Narsil/image_dummy",
"image",
split="test",
)
data = self.read(dataset["file"][0])
result = api(data=data)
self.assertIsInstance(result, list)
Expand Down
2 changes: 1 addition & 1 deletion tests/test_keras_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ def test_push_to_hub_keras_mixin_via_http_basic(self):
)

# Test model id exists
model_info = self._api.model_info(repo_id, token=self._token)
model_info = self._api.model_info(repo_id, use_auth_token=self._token)
self.assertEqual(model_info.modelId, repo_id)

# Test config has been pushed to hub
Expand Down

0 comments on commit 55bb9ec

Please sign in to comment.