From 3eed264bade0e0109c83f24b5804bb85f1412929 Mon Sep 17 00:00:00 2001 From: "mulin.lyh" Date: Fri, 8 Jul 2022 20:03:23 +0800 Subject: [PATCH] [to #43040150] fix: login warn refactor, error message when exception, ut case to new user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 未登录warn提示信息重构，只有遇到请求失败，exception时，如果cookie为空，提示用户login，单元测试用户修改成单独 Link: https://code.alibaba-inc.com/Ali-MaaS/MaaS-lib/codereview/9283579 * [to #42322933] add space dialog-state tracking pipeline Link: https://code.alibaba-inc.com/Ali-MaaS/MaaS-lib/codereview/9227018 * init * token to ids * add model * model forward ready * add intent * intent preprocessor ready * intent success * merge master * test with model hub * add flake8 * update * update * update * Merge branch 'master' into nlp/space/gen * delete file about gen * init * fix flake8 bug * [to #42322933] init * bug fix * [to #42322933] init * update pipeline registry info * Merge remote-tracking branch 'origin/master' into feat/nli * [to #42322933] init * [to #42322933] init * modify forward * [to #42322933] init * generation ready * init * Merge branch 'master' into feat/zero_shot_classification # Conflicts: # modelscope/preprocessors/__init__.py * [to #42322933] bugfix * [to #42322933] pre commit fix * fill mask * registry multi models on model and pipeline * add tests * test level >= 0 * local gen ready * merge with master * dialog modeling ready * fix comments: rename and refactor AliceMindMLM; adjust pipeline * space intent and modeling(generation) are ready * bug fix * add dep * add dep * support dst data processor * merge with nlp/space/dst * merge with master * Merge remote-tracking branch 'origin' into feat/fill_mask Conflicts: modelscope/models/nlp/__init__.py modelscope/pipelines/builder.py modelscope/pipelines/outputs.py modelscope/preprocessors/nlp.py requirements/nlp.txt * merge with master * merge with master 2/2 * fix comments * fix isort for pre-commit check * allow params pass to pipeline's __call__ method * Merge remote-tracking branch 'origin/master' into feat/zero_shot_classification * merge with nli task * merge with sentiment_classification * merge with zero_shot_classfication * merge with fill_mask * merge with space * merge with master head * Merge remote-tracking branch 'origin' into feat/fill_mask Conflicts: modelscope/utils/constant.py * fix: pipeline module_name from model_type to 'fill_mask' & fix merge bug * unfiinished change * fix bug * unfinished * unfinished * revise modelhub dependency * Merge branch 'feat/nlp_refactor' of http://gitlab.alibaba-inc.com/Ali-MaaS/MaaS-lib into feat/nlp_refactor * add eval() to pipeline call * add test level * ut run passed * add default args * tmp * merge master * all ut passed * remove an useless enum * revert a mis modification * revert a mis modification * Merge commit 'ace8af92465f7d772f035aebe98967726655f12c' into feat/nlp * commit 'ace8af92465f7d772f035aebe98967726655f12c': [to #42322933] Add cv-action-recongnition-pipeline to maas lib [to #42463204] support Pil.Image for image_captioning_pipeline [to #42670107] restore pydataset test [to #42322933] add create if not exist and add(back) create model example Link: https://code.alibaba-inc.com/Ali-MaaS/MaaS-lib/codereview/9130661 [to #41474818]fix: fix errors in task name definition # Conflicts: # modelscope/pipelines/builder.py # modelscope/utils/constant.py * Merge branch 'feat/nlp' into feat/nlp_refactor * feat/nlp: [to #42322933] Add cv-action-recongnition-pipeline to maas lib [to #42463204] support Pil.Image for image_captioning_pipeline [to #42670107] restore pydataset test [to #42322933] add create if not exist and add(back) create model example Link: https://code.alibaba-inc.com/Ali-MaaS/MaaS-lib/codereview/9130661 [to #41474818]fix: fix errors in task name definition # Conflicts: # modelscope/pipelines/builder.py * fix compile bug * refactor space * Merge branch 'feat/nlp_refactor' of http://gitlab.alibaba-inc.com/Ali-MaaS/MaaS-lib into feat/nlp_refactor * Merge remote-tracking branch 'origin' into feat/fill_mask * fix * pre-commit lint * lint file * lint file * lint file * update modelhub dependency * lint file * ignore dst_processor temporary * solve comment: 1. change MaskedLMModelBase to MaskedLanguageModelBase 2. remove a useless import * recommit * remove MaskedLanguageModel from __all__ * Merge commit '1a0d4af55a2eee69d89633874890f50eda8f8700' into feat/nlp_refactor * commit '1a0d4af55a2eee69d89633874890f50eda8f8700': [to #42322933] test level check Link: https://code.alibaba-inc.com/Ali-MaaS/MaaS-lib/codereview/9143809 [to #42322933] update nlp models name in metainfo Link: https://code.alibaba-inc.com/Ali-MaaS/MaaS-lib/codereview/9134657 # Conflicts: # modelscope/metainfo.py * update * revert pipeline params update * remove zeroshot * update sequence classfication outpus * merge with fill mask * Merge remote-tracking branch 'origin' into feat/fill_mask * fix * init dialog state tracking * fix flake8 warning of dst * Merge remote-tracking branch 'origin/feat/fill_mask' into feat/nlp * merge with master * remove useless test.py * add init * merge nlp * Merge remote-tracking branch 'origin/master' into feat/nlp * remove unformatted space trainer * Merge branch 'feat/nlp' into nlp/space/dst * revise based on comment except chinease comment * skip ci blocking * change Chinese notes of space3.0 into English * translate chinese comment to english * add space to metainfo * space dst pipeline is ready, but model's result is wrong * merge feat/nlp * merge with master * change processor * change example * test case ready * dst loacl ready * update dst conf * merge feat/nlp * inform revise * inherit bug fix * init * add 2 complete examples * fix bug * add test case * merge with master * modify model name * add missing setting * add outputs * modify test level * modify chinese comment * remove useless doc * merge feat/nlp * Merge remote-tracking branch 'origin' into nlp/space/dst * Merge branch 'feat/nlp' into nlp/space/dst * dst test ready * merge feat nlp * space outputs normalization * update dst * merge feat nlp * Merge branch 'master' into nlp/space/dst * update requirement * merge with master * Merge remote-tracking branch 'origin/master' into nlp/space/dst * formating output * update requirements/nlp * merge with master * add test cases * Merge remote-tracking branch 'origin/master' into nlp/space/dst * merge with master * login warn refactor, error message when exception, ut case to new user * login warn refactor, error message when exception, ut case to new user --- modelscope/hub/api.py | 20 ++++++------- modelscope/hub/errors.py | 14 +++++++++ tests/hub/test_hub_operation.py | 15 ++++------ tests/hub/test_hub_private_files.py | 35 ++++++++++++++--------- tests/hub/test_hub_private_repository.py | 18 +++++------- tests/hub/test_hub_repository.py | 36 ++++++------------------ tests/hub/test_utils.py | 27 ++++++++++++++++++ 7 files changed, 93 insertions(+), 72 deletions(-) create mode 100644 tests/hub/test_utils.py diff --git a/modelscope/hub/api.py b/modelscope/hub/api.py index 6aeedc02..d847bf65 100644 --- a/modelscope/hub/api.py +++ b/modelscope/hub/api.py @@ -14,7 +14,7 @@ from ..msdatasets.config import DOWNLOADED_DATASETS_PATH, HUB_DATASET_ENDPOINT from ..utils.constant import DownloadMode from .constants import MODELSCOPE_URL_SCHEME from .errors import (InvalidParameter, NotExistError, datahub_raise_on_error, - is_ok, raise_on_error) + handle_http_response, is_ok, raise_on_error) from .utils.utils import get_endpoint, model_id_to_group_owner_name logger = get_logger() @@ -121,6 +121,8 @@ class HubApi: """ cookies = ModelScopeConfig.get_cookies() + if cookies is None: + raise ValueError('Token does not exist, please login first.') path = f'{self.endpoint}/api/v1/models/{model_id}' r = requests.delete(path, cookies=cookies) @@ -154,6 +156,7 @@ class HubApi: path = f'{self.endpoint}/api/v1/models/{owner_or_group}/{name}?{revision}' r = requests.get(path, cookies=cookies) + handle_http_response(r, logger, cookies, model_id) if r.status_code == 200: if is_ok(r.json()): return r.json()['Data'] @@ -192,7 +195,7 @@ class HubApi: path = f'{self.endpoint}/api/v1/models/{model_id}/revisions' r = requests.get(path, cookies=cookies) - r.raise_for_status() + handle_http_response(r, logger, cookies, model_id) d = r.json() raise_on_error(d) info = d['Data'] @@ -234,7 +237,7 @@ class HubApi: r = requests.get(path, cookies=cookies, headers=headers) - r.raise_for_status() + handle_http_response(r, logger, cookies, model_id) d = r.json() raise_on_error(d) @@ -326,19 +329,16 @@ class ModelScopeConfig: @classmethod def get_cookies(cls): - try: - cookies_path = os.path.join(cls.path_credential, 'cookies') + cookies_path = os.path.join(cls.path_credential, 'cookies') + if os.path.exists(cookies_path): with open(cookies_path, 'rb') as f: cookies = pickle.load(f) for cookie in cookies: if cookie.is_expired(): - logger.warn('Auth is expored, please re-login') + logger.warn( + 'Authentication has expired, please re-login') return None return cookies - except FileNotFoundError: - logger.warn( - "Auth token does not exist, you'll get authentication error when downloading \ - private model files. Please login first") return None @classmethod diff --git a/modelscope/hub/errors.py b/modelscope/hub/errors.py index 9a19fdb5..3fffeeda 100644 --- a/modelscope/hub/errors.py +++ b/modelscope/hub/errors.py @@ -1,3 +1,6 @@ +from requests.exceptions import HTTPError + + class NotExistError(Exception): pass @@ -26,6 +29,17 @@ def is_ok(rsp): return rsp['Code'] == 200 and rsp['Success'] +def handle_http_response(response, logger, cookies, model_id): + try: + response.raise_for_status() + except HTTPError: + if cookies is None: # code in [403] and + logger.error( + f'Authentication token does not exist, failed to access model {model_id} which may be private. \ + Please login first.') + raise + + def raise_on_error(rsp): """If response error, raise exception diff --git a/tests/hub/test_hub_operation.py b/tests/hub/test_hub_operation.py index ea83fa2a..d661199f 100644 --- a/tests/hub/test_hub_operation.py +++ b/tests/hub/test_hub_operation.py @@ -12,12 +12,9 @@ from modelscope.hub.constants import Licenses, ModelVisibility from modelscope.hub.file_download import model_file_download from modelscope.hub.repository import Repository from modelscope.hub.snapshot_download import snapshot_download +from .test_utils import (TEST_MODEL_CHINESE_NAME, TEST_MODEL_ORG, + TEST_PASSWORD, TEST_USER_NAME1) -USER_NAME = 'maasadmin' -PASSWORD = '12345678' - -model_chinese_name = '达摩卡通化模型' -model_org = 'unittest' DEFAULT_GIT_PATH = 'git' download_model_file_name = 'test.bin' @@ -28,14 +25,14 @@ class HubOperationTest(unittest.TestCase): def setUp(self): self.api = HubApi() # note this is temporary before official account management is ready - self.api.login(USER_NAME, PASSWORD) + self.api.login(TEST_USER_NAME1, TEST_PASSWORD) self.model_name = uuid.uuid4().hex - self.model_id = '%s/%s' % (model_org, self.model_name) + self.model_id = '%s/%s' % (TEST_MODEL_ORG, self.model_name) self.api.create_model( model_id=self.model_id, visibility=ModelVisibility.PUBLIC, license=Licenses.APACHE_V2, - chinese_name=model_chinese_name, + chinese_name=TEST_MODEL_CHINESE_NAME, ) temporary_dir = tempfile.mkdtemp() self.model_dir = os.path.join(temporary_dir, self.model_name) @@ -97,7 +94,7 @@ class HubOperationTest(unittest.TestCase): file_path=download_model_file_name, cache_dir=temporary_dir) assert os.path.exists(downloaded_file) - self.api.login(USER_NAME, PASSWORD) + self.api.login(TEST_USER_NAME1, TEST_PASSWORD) def test_snapshot_delete_download_cache_file(self): snapshot_path = snapshot_download(model_id=self.model_id) diff --git a/tests/hub/test_hub_private_files.py b/tests/hub/test_hub_private_files.py index b9c71456..61048791 100644 --- a/tests/hub/test_hub_private_files.py +++ b/tests/hub/test_hub_private_files.py @@ -13,13 +13,9 @@ from modelscope.hub.file_download import model_file_download from modelscope.hub.repository import Repository from modelscope.hub.snapshot_download import snapshot_download from modelscope.utils.constant import ModelFile - -USER_NAME = 'maasadmin' -PASSWORD = '12345678' -USER_NAME2 = 'sdkdev' - -model_chinese_name = '达摩卡通化模型' -model_org = 'unittest' +from .test_utils import (TEST_MODEL_CHINESE_NAME, TEST_MODEL_ORG, + TEST_PASSWORD, TEST_USER_NAME1, TEST_USER_NAME2, + delete_credential) class HubPrivateFileDownloadTest(unittest.TestCase): @@ -28,17 +24,20 @@ class HubPrivateFileDownloadTest(unittest.TestCase): self.old_cwd = os.getcwd() self.api = HubApi() # note this is temporary before official account management is ready - self.token, _ = self.api.login(USER_NAME, PASSWORD) + self.token, _ = self.api.login(TEST_USER_NAME1, TEST_PASSWORD) self.model_name = uuid.uuid4().hex - self.model_id = '%s/%s' % (model_org, self.model_name) + self.model_id = '%s/%s' % (TEST_MODEL_ORG, self.model_name) self.api.create_model( model_id=self.model_id, visibility=ModelVisibility.PRIVATE, # 1-private, 5-public license=Licenses.APACHE_V2, - chinese_name=model_chinese_name, + chinese_name=TEST_MODEL_CHINESE_NAME, ) def tearDown(self): + # credential may deleted or switch login name, we need re-login here + # to ensure the temporary model is deleted. + self.api.login(TEST_USER_NAME1, TEST_PASSWORD) os.chdir(self.old_cwd) self.api.delete_model(model_id=self.model_id) @@ -47,20 +46,28 @@ class HubPrivateFileDownloadTest(unittest.TestCase): assert os.path.exists(os.path.join(snapshot_path, ModelFile.README)) def test_snapshot_download_private_model_no_permission(self): - self.token, _ = self.api.login(USER_NAME2, PASSWORD) + self.token, _ = self.api.login(TEST_USER_NAME2, TEST_PASSWORD) + with self.assertRaises(HTTPError): + snapshot_download(self.model_id) + + def test_snapshot_download_private_model_without_login(self): + delete_credential() with self.assertRaises(HTTPError): snapshot_download(self.model_id) - self.api.login(USER_NAME, PASSWORD) def test_download_file_private_model(self): file_path = model_file_download(self.model_id, ModelFile.README) assert os.path.exists(file_path) def test_download_file_private_model_no_permission(self): - self.token, _ = self.api.login(USER_NAME2, PASSWORD) + self.token, _ = self.api.login(TEST_USER_NAME2, TEST_PASSWORD) + with self.assertRaises(HTTPError): + model_file_download(self.model_id, ModelFile.README) + + def test_download_file_private_model_without_login(self): + delete_credential() with self.assertRaises(HTTPError): model_file_download(self.model_id, ModelFile.README) - self.api.login(USER_NAME, PASSWORD) def test_snapshot_download_local_only(self): with self.assertRaises(ValueError): diff --git a/tests/hub/test_hub_private_repository.py b/tests/hub/test_hub_private_repository.py index 01a89586..132a1c5a 100644 --- a/tests/hub/test_hub_private_repository.py +++ b/tests/hub/test_hub_private_repository.py @@ -8,13 +8,9 @@ from modelscope.hub.api import HubApi from modelscope.hub.constants import Licenses, ModelVisibility from modelscope.hub.errors import GitError from modelscope.hub.repository import Repository +from .test_utils import (TEST_MODEL_CHINESE_NAME, TEST_MODEL_ORG, + TEST_PASSWORD, TEST_USER_NAME1, TEST_USER_NAME2) -USER_NAME = 'maasadmin' -PASSWORD = '12345678' - -USER_NAME2 = 'sdkdev' -model_chinese_name = '达摩卡通化模型' -model_org = 'unittest' DEFAULT_GIT_PATH = 'git' @@ -24,23 +20,23 @@ class HubPrivateRepositoryTest(unittest.TestCase): self.old_cwd = os.getcwd() self.api = HubApi() # note this is temporary before official account management is ready - self.token, _ = self.api.login(USER_NAME, PASSWORD) + self.token, _ = self.api.login(TEST_USER_NAME1, TEST_PASSWORD) self.model_name = uuid.uuid4().hex - self.model_id = '%s/%s' % (model_org, self.model_name) + self.model_id = '%s/%s' % (TEST_MODEL_ORG, self.model_name) self.api.create_model( model_id=self.model_id, visibility=ModelVisibility.PRIVATE, # 1-private, 5-public license=Licenses.APACHE_V2, - chinese_name=model_chinese_name, + chinese_name=TEST_MODEL_CHINESE_NAME, ) def tearDown(self): - self.api.login(USER_NAME, PASSWORD) + self.api.login(TEST_USER_NAME1, TEST_PASSWORD) os.chdir(self.old_cwd) self.api.delete_model(model_id=self.model_id) def test_clone_private_repo_no_permission(self): - token, _ = self.api.login(USER_NAME2, PASSWORD) + token, _ = self.api.login(TEST_USER_NAME2, TEST_PASSWORD) temporary_dir = tempfile.mkdtemp() local_dir = os.path.join(temporary_dir, self.model_name) with self.assertRaises(GitError) as cm: diff --git a/tests/hub/test_hub_repository.py b/tests/hub/test_hub_repository.py index cf9f4403..48730df6 100644 --- a/tests/hub/test_hub_repository.py +++ b/tests/hub/test_hub_repository.py @@ -15,48 +15,28 @@ from modelscope.hub.file_download import model_file_download from modelscope.hub.git import GitCommandWrapper from modelscope.hub.repository import Repository from modelscope.utils.logger import get_logger +from .test_utils import (TEST_MODEL_CHINESE_NAME, TEST_MODEL_ORG, + TEST_PASSWORD, TEST_USER_NAME1, TEST_USER_NAME2, + delete_credential, delete_stored_git_credential) logger = get_logger() logger.setLevel('DEBUG') -USER_NAME = 'maasadmin' -PASSWORD = '12345678' - -model_chinese_name = '达摩卡通化模型' -model_org = 'unittest' DEFAULT_GIT_PATH = 'git' -def delete_credential(): - path_credential = expanduser('~/.modelscope/credentials') - shutil.rmtree(path_credential) - - -def delete_stored_git_credential(user): - credential_path = expanduser('~/.git-credentials') - if os.path.exists(credential_path): - with open(credential_path, 'r+') as f: - lines = f.readlines() - for line in lines: - if user in line: - lines.remove(line) - f.seek(0) - f.write(''.join(lines)) - f.truncate() - - class HubRepositoryTest(unittest.TestCase): def setUp(self): self.api = HubApi() # note this is temporary before official account management is ready - self.api.login(USER_NAME, PASSWORD) + self.api.login(TEST_USER_NAME1, TEST_PASSWORD) self.model_name = uuid.uuid4().hex - self.model_id = '%s/%s' % (model_org, self.model_name) + self.model_id = '%s/%s' % (TEST_MODEL_ORG, self.model_name) self.api.create_model( model_id=self.model_id, visibility=ModelVisibility.PUBLIC, # 1-private, 5-public license=Licenses.APACHE_V2, - chinese_name=model_chinese_name, + chinese_name=TEST_MODEL_CHINESE_NAME, ) temporary_dir = tempfile.mkdtemp() self.model_dir = os.path.join(temporary_dir, self.model_name) @@ -70,10 +50,10 @@ class HubRepositoryTest(unittest.TestCase): def test_clone_public_model_without_token(self): delete_credential() - delete_stored_git_credential(USER_NAME) + delete_stored_git_credential(TEST_USER_NAME1) Repository(self.model_dir, clone_from=self.model_id) assert os.path.exists(os.path.join(self.model_dir, 'README.md')) - self.api.login(USER_NAME, PASSWORD) # re-login for delete + self.api.login(TEST_USER_NAME1, TEST_PASSWORD) # re-login for delete def test_push_all(self): repo = Repository(self.model_dir, clone_from=self.model_id) diff --git a/tests/hub/test_utils.py b/tests/hub/test_utils.py new file mode 100644 index 00000000..7124d13e --- /dev/null +++ b/tests/hub/test_utils.py @@ -0,0 +1,27 @@ +import os +import shutil +from codecs import ignore_errors +from os.path import expanduser + +TEST_USER_NAME1 = 'citest' +TEST_USER_NAME2 = 'sdkdev' +TEST_PASSWORD = '12345678' + +TEST_MODEL_CHINESE_NAME = '内部测试模型' +TEST_MODEL_ORG = 'citest' + + +def delete_credential(): + path_credential = expanduser('~/.modelscope/credentials') + shutil.rmtree(path_credential, ignore_errors=True) + + +def delete_stored_git_credential(user): + credential_path = expanduser('~/.git-credentials') + if os.path.exists(credential_path): + with open(credential_path, 'r+') as f: + lines = f.readlines() + lines = [line for line in lines if user not in line] + f.seek(0) + f.write(''.join(lines)) + f.truncate()