Skip to content

Commit

Permalink
CI Error on FutureWarning (#787)
Browse files Browse the repository at this point in the history
* error on FutureWarning

* login -> set_access_token

* add missing USER import

* set_access_token returns nothing

* minor fixes

* more fixes

* snapshot test fix
  • Loading branch information
adrinjalali authored Mar 24, 2022
1 parent b352e99 commit f9073fe
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 36 deletions.
10 changes: 5 additions & 5 deletions .github/workflows/python-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ jobs:
- run: |
if [[ "${{ matrix.test_repository }}" == "Repository only" ]]
then
pytest --log-cli-level=INFO -sv ./tests/test_repository.py -k RepositoryTest
pytest -Werror::FutureWarning --log-cli-level=INFO -sv ./tests/test_repository.py -k RepositoryTest
else
pytest --log-cli-level=INFO -sv ./tests/ -k 'not RepositoryTest'
pytest -Werror::FutureWarning --log-cli-level=INFO -sv ./tests/ -k 'not RepositoryTest'
fi
build_pytorch:
Expand All @@ -67,7 +67,7 @@ jobs:
pip install --upgrade pip
pip install .[testing,torch]
- run: pytest -sv ./tests/test_hubmixin*
- run: pytest -Werror::FutureWarning -sv ./tests/test_hubmixin*

build_tensorflow:
runs-on: ubuntu-latest
Expand All @@ -89,7 +89,7 @@ jobs:
pip install --upgrade pip
pip install .[testing,tensorflow]
- run: pytest -sv ./tests/test_keras*
- run: pytest -Werror::FutureWarning -sv ./tests/test_keras*

tests_lfs:
runs-on: ubuntu-latest
Expand All @@ -111,4 +111,4 @@ jobs:
pip install --upgrade pip
pip install .[testing]
- run: RUN_GIT_LFS_TESTS=1 pytest -sv ./tests/ -k "HfLargefilesTest"
- run: RUN_GIT_LFS_TESTS=1 pytest -Werror::FutureWarning -sv ./tests/ -k "HfLargefilesTest"
6 changes: 4 additions & 2 deletions src/huggingface_hub/hf_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -1378,7 +1378,8 @@ def upload_file(
The repository to which the file will be uploaded, for example: :obj:`"username/custom_transformers"`
token (``str``):
Authentication token, obtained with :function:`HfApi.login` method. Will default to the stored token.
Authentication token which can be obtained from
https://huggingface.co/settings/tokens.
repo_type (``str``, Optional):
Set to :obj:`"dataset"` or :obj:`"space"` if uploading to a dataset or space, :obj:`None` or :obj:`"model"` if uploading to a model. Default is :obj:`None`.
Expand Down Expand Up @@ -1501,7 +1502,8 @@ def delete_file(
The repository from which the file will be deleted, for example: :obj:`"username/custom_transformers"`
token (``str``):
Authentication token, obtained with :function:`HfApi.login` method. Will default to the stored token.
Authentication token which can be obtained from
https://huggingface.co/settings/tokens.
repo_type (``str``, Optional):
Set to :obj:`"dataset"` or :obj:`"space"` if the file is in a dataset or space, :obj:`None` or :obj:`"model"` if in a model. Default is :obj:`None`.
Expand Down
36 changes: 20 additions & 16 deletions tests/test_hf_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,25 +113,30 @@ def setUp(self) -> None:

@classmethod
def tearDownClass(cls) -> None:
cls._api.login(username=USER, password=PASS)
with pytest.warns(FutureWarning, match="This method is deprecated"):
cls._api.login(username=USER, password=PASS)

def test_login_invalid(self):
with self.assertRaises(HTTPError):
self._api.login(username=USER, password="fake")
with pytest.warns(FutureWarning, match="This method is deprecated"):
with pytest.raises(HTTPError):
self._api.login(username=USER, password="fake")

def test_login_valid(self):
token = self._api.login(username=USER, password=PASS)
self.assertIsInstance(token, str)
with pytest.warns(FutureWarning, match="This method is deprecated"):
token = self._api.login(username=USER, password=PASS)
assert isinstance(token, str)

def test_login_git_credentials(self):
self.assertTupleEqual(read_from_credential_store(USER), (None, None))
self._api.login(username=USER, password=PASS)
with pytest.warns(FutureWarning, match="This method is deprecated"):
self._api.login(username=USER, password=PASS)
self.assertTupleEqual(read_from_credential_store(USER), (USER.lower(), PASS))
erase_from_credential_store(username=USER)
self.assertTupleEqual(read_from_credential_store(USER), (None, None))

def test_login_cli(self):
_login(self._api, username=USER, password=PASS)
with pytest.warns(FutureWarning, match="This method is deprecated"):
_login(self._api, username=USER, password=PASS)
self.assertTupleEqual(read_from_credential_store(USER), (USER.lower(), PASS))
erase_from_credential_store(username=USER)
self.assertTupleEqual(read_from_credential_store(USER), (None, None))
Expand Down Expand Up @@ -172,15 +177,15 @@ def setUpClass(cls):
"""
Share this valid token in all tests below.
"""
cls._token = cls._api.login(username=USER, password=PASS)
cls._token = TOKEN
cls._api.set_access_token(TOKEN)


@retry_endpoint
def test_repo_id_no_warning():
# tests that passing repo_id as positional arg doesn't raise any warnings
# for {create, delete}_repo and update_repo_visibility
api = HfApi(endpoint=ENDPOINT_STAGING)
token = api.login(username=USER, password=PASS)
REPO_NAME = repo_name("crud")

args = [
Expand All @@ -192,7 +197,7 @@ def test_repo_id_no_warning():
for method, kwargs in args:
with warnings.catch_warnings(record=True) as record:
getattr(api, method)(
REPO_NAME, token=token, repo_type=REPO_TYPE_MODEL, **kwargs
REPO_NAME, token=TOKEN, repo_type=REPO_TYPE_MODEL, **kwargs
)
assert not len(record)

Expand Down Expand Up @@ -222,7 +227,6 @@ def test_name_org_deprecation_warning():
# test that the right warning is raised when passing name to
# {create, delete}_repo and update_repo_visibility
api = HfApi(endpoint=ENDPOINT_STAGING)
token = api.login(username=USER, password=PASS)
REPO_NAME = repo_name("crud")

args = [
Expand All @@ -237,7 +241,7 @@ def test_name_org_deprecation_warning():
match=re.escape("`name` and `organization` input arguments are deprecated"),
):
getattr(api, method)(
name=REPO_NAME, token=token, repo_type=REPO_TYPE_MODEL, **kwargs
name=REPO_NAME, token=TOKEN, repo_type=REPO_TYPE_MODEL, **kwargs
)


Expand All @@ -246,7 +250,6 @@ def test_name_org_deprecation_error():
# tests that the right error is raised when passing both name and repo_id
# to {create, delete}_repo and update_repo_visibility
api = HfApi(endpoint=ENDPOINT_STAGING)
token = api.login(username=USER, password=PASS)
REPO_NAME = repo_name("crud")

args = [
Expand All @@ -263,14 +266,14 @@ def test_name_org_deprecation_error():
getattr(api, method)(
repo_id="test",
name=REPO_NAME,
token=token,
token=TOKEN,
repo_type=REPO_TYPE_MODEL,
**kwargs,
)

for method, kwargs in args:
with pytest.raises(ValueError, match="No name provided"):
getattr(api, method)(token=token, repo_type=REPO_TYPE_MODEL, **kwargs)
getattr(api, method)(token=TOKEN, repo_type=REPO_TYPE_MODEL, **kwargs)


class HfApiEndpointsTest(HfApiCommonTestWithLogin):
Expand Down Expand Up @@ -1108,7 +1111,8 @@ def setUpClass(cls):
"""
Share this valid token in all tests below.
"""
cls._token = cls._api.login(username=USER, password=PASS)
cls._token = TOKEN
cls._api.set_access_token(TOKEN)

def setUp(self):
self.REPO_NAME_LARGE_FILE = repo_name_large_file()
Expand Down
5 changes: 3 additions & 2 deletions tests/test_hubmixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from huggingface_hub.hub_mixin import PyTorchModelHubMixin
from huggingface_hub.utils import logging

from .testing_constants import ENDPOINT_STAGING, PASS, USER
from .testing_constants import ENDPOINT_STAGING, TOKEN, USER
from .testing_utils import set_write_permission_and_retry


Expand Down Expand Up @@ -74,7 +74,8 @@ def setUpClass(cls):
"""
Share this valid token in all tests below.
"""
cls._token = cls._api.login(username=USER, password=PASS)
cls._token = TOKEN
cls._api.set_access_token(TOKEN)

def test_save_pretrained(self):
REPO_NAME = repo_name("save")
Expand Down
5 changes: 3 additions & 2 deletions tests/test_keras_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
)
from huggingface_hub.utils import logging

from .testing_constants import ENDPOINT_STAGING, PASS, USER
from .testing_constants import ENDPOINT_STAGING, TOKEN, USER
from .testing_utils import retry_endpoint, set_write_permission_and_retry


Expand Down Expand Up @@ -85,7 +85,8 @@ def setUpClass(cls):
Share this valid token in all tests below.
"""
cls._api = HfApi(endpoint=ENDPOINT_STAGING)
cls._token = cls._api.login(username=USER, password=PASS)
cls._token = TOKEN
cls._api.set_access_token(TOKEN)

def test_save_pretrained(self):
REPO_NAME = repo_name("save")
Expand Down
14 changes: 9 additions & 5 deletions tests/test_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
)
from huggingface_hub.utils import logging

from .testing_constants import ENDPOINT_STAGING, PASS, USER
from .testing_constants import ENDPOINT_STAGING, TOKEN, USER
from .testing_utils import (
retry_endpoint,
set_write_permission_and_retry,
Expand Down Expand Up @@ -69,7 +69,8 @@ def setUpClass(cls):
"""
Share this valid token in all tests below.
"""
cls._token = cls._api.login(username=USER, password=PASS)
cls._api.set_access_token(TOKEN)
cls._token = TOKEN

@retry_endpoint
def setUp(self):
Expand Down Expand Up @@ -1328,7 +1329,8 @@ def test_repo_init_checkout_revision(self):

def test_repo_user(self):
api = HfApi(endpoint=ENDPOINT_STAGING)
token = api.login(USER, PASS)
token = TOKEN
api.set_access_token(TOKEN)

repo = Repository(WORKING_REPO_DIR, use_auth_token=token)
user = api.whoami(token)
Expand All @@ -1355,7 +1357,8 @@ def test_repo_user(self):

def test_repo_passed_user(self):
api = HfApi(endpoint=ENDPOINT_STAGING)
token = api.login(USER, PASS)
token = TOKEN
api.set_access_token(TOKEN)
repo = Repository(
WORKING_REPO_DIR,
git_user="RANDOM_USER",
Expand Down Expand Up @@ -1461,7 +1464,8 @@ def setUpClass(cls):
"""
Share this valid token in all tests below.
"""
cls._token = cls._api.login(username=USER, password=PASS)
cls._token = TOKEN
cls._api.set_access_token(TOKEN)

def setUp(self):
self.REPO_NAME = repo_name()
Expand Down
9 changes: 5 additions & 4 deletions tests/test_snapshot_download.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from huggingface_hub.snapshot_download import snapshot_download
from huggingface_hub.utils import logging

from .testing_constants import ENDPOINT_STAGING, PASS, USER
from .testing_constants import ENDPOINT_STAGING, TOKEN, USER
from .testing_utils import retry_endpoint, set_write_permission_and_retry


Expand All @@ -27,7 +27,8 @@ def setUpClass(cls):
"""
Share this valid token in all tests below.
"""
cls._token = cls._api.login(username=USER, password=PASS)
cls._token = TOKEN
cls._api.set_access_token(TOKEN)

@retry_endpoint
def setUp(self) -> None:
Expand Down Expand Up @@ -110,7 +111,7 @@ def test_download_model(self):

def test_download_private_model(self):
self._api.update_repo_visibility(
token=self._token, name=REPO_NAME, private=True
token=self._token, repo_id=REPO_NAME, private=True
)

# Test download fails without token
Expand Down Expand Up @@ -170,7 +171,7 @@ def test_download_private_model(self):
self.assertTrue(self.second_commit_hash in storage_folder)

self._api.update_repo_visibility(
token=self._token, name=REPO_NAME, private=False
token=self._token, repo_id=REPO_NAME, private=False
)

def test_download_model_local_only(self):
Expand Down

0 comments on commit f9073fe

Please sign in to comment.