From 37136f37448ebb237fdaef2befc981c605ab0d17 Mon Sep 17 00:00:00 2001 From: "Xingjun.Wang" Date: Tue, 1 Sep 2026 16:05:47 +0800 Subject: [PATCH] [Fix] Unified upload envs (#1787) --- modelscope/hub/constants.py | 69 +++---------- modelscope/hub/mcp_api.py | 47 ++++----- modelscope/hub/push_to_hub.py | 5 +- modelscope/hub/utils/aigc.py | 26 ++++- requirements/hub.txt | 2 +- tests/hub/test_create_aigc_model.py | 153 +++++++++++++++++++++++----- tests/mcp/test_mcp_api.py | 38 +++++++ 7 files changed, 225 insertions(+), 115 deletions(-) diff --git a/modelscope/hub/constants.py b/modelscope/hub/constants.py index 28838ec3..9ae5afe6 100644 --- a/modelscope/hub/constants.py +++ b/modelscope/hub/constants.py @@ -13,8 +13,17 @@ from modelscope_hub.compat.constants import ( # noqa: F401 DEFAULT_DATASET_REVISION, DEFAULT_MAX_WORKERS, FILE_HASH, MODELSCOPE_DOMAIN, MODELSCOPE_PREFER_AI_SITE, REPO_TYPE_DATASET, REPO_TYPE_MODEL, REPO_TYPE_STUDIO, REPO_TYPE_SUPPORT, - TEMPORARY_FOLDER_NAME, ModelVisibility_INTERNAL, ModelVisibility_PRIVATE, - ModelVisibility_PUBLIC) + TEMPORARY_FOLDER_NAME, UPLOAD_ADAPTIVE_BATCH_SIZE, UPLOAD_BLOB_MAX_RETRIES, + UPLOAD_BLOB_RETRY_BACKOFF, UPLOAD_BLOB_RETRY_MAX_WAIT, UPLOAD_BLOB_TIMEOUT, + UPLOAD_BLOB_TQDM_DISABLE_THRESHOLD, UPLOAD_COMMIT_BATCH_SIZE, + UPLOAD_FAILED_FILE_MAX_RETRIES, UPLOAD_MAX_FILE_COUNT, + UPLOAD_MAX_FILE_COUNT_IN_DIR, UPLOAD_MAX_FILE_SIZE, + UPLOAD_NORMAL_FILE_SIZE_TOTAL_LIMIT, UPLOAD_REACT_BACKOFF_MAX_EXPONENT, + UPLOAD_REACT_ENABLED, UPLOAD_REACT_MAX_DELAY, + UPLOAD_REACT_ROUND2_BASE_DELAY, UPLOAD_REACT_ROUND3_FILE_DELAY, + UPLOAD_RETRY_ALLOWED_METHODS, UPLOAD_SIZE_THRESHOLD_TO_ENFORCE_LFS, + UPLOAD_USE_CACHE, UPLOAD_VALIDATE_BLOB_BATCH_SIZE, + ModelVisibility_INTERNAL, ModelVisibility_PRIVATE, ModelVisibility_PUBLIC) # --- Local constants (not in modelscope_hub) --- MODELSCOPE_URL_SCHEME = 'https://' @@ -50,30 +59,8 @@ CREATE_TAG_MAX_RETRIES = int( CREATE_TAG_RETRY_BACKOFF = int( os.environ.get('MODELSCOPE_CREATE_TAG_RETRY_BACKOFF', 2)) -# Application-level retry for blob upload -UPLOAD_BLOB_MAX_RETRIES = int(os.environ.get('UPLOAD_BLOB_MAX_RETRIES', 5)) -UPLOAD_BLOB_RETRY_BACKOFF = int(os.environ.get('UPLOAD_BLOB_RETRY_BACKOFF', 2)) -UPLOAD_BLOB_RETRY_MAX_WAIT = int( - os.environ.get('UPLOAD_BLOB_RETRY_MAX_WAIT', 60)) - -# Failed file retry within upload_folder -UPLOAD_FAILED_FILE_MAX_RETRIES = int( - os.environ.get('UPLOAD_FAILED_FILE_MAX_RETRIES', 3)) - -# Per-socket-operation timeout for blob upload (not total transfer time). -# Each individual socket send/recv must complete within this limit. -# Safe for any file size: a 100GB upload's total time can exceed this, -# but each chunk I/O operation finishes in milliseconds. -UPLOAD_BLOB_TIMEOUT = (30, - int( - os.environ.get('UPLOAD_BLOB_TIMEOUT_SECONDS', - 3600))) - -# Methods that are safe for urllib3 transport-level retry. -# PUT is excluded because blob uploads use streaming data that cannot be replayed. -UPLOAD_RETRY_ALLOWED_METHODS = frozenset( - os.environ.get('UPLOAD_RETRY_ALLOWED_METHODS', - 'GET,HEAD,DELETE,OPTIONS,TRACE').split(',')) +# Upload runtime configuration is owned by modelscope-hub and re-exported +# above under the historical modelscope constant names. API_RESPONSE_FIELD_DATA = 'Data' API_FILE_DOWNLOAD_RETRY_TIMES = 5 @@ -91,36 +78,6 @@ ONE_YEAR_SECONDS = 24 * 365 * 60 * 60 MODELSCOPE_REQUEST_ID = 'X-Request-ID' DEFAULT_SKILLS_DIR = os.path.join(os.path.expanduser('~'), '.agents', 'skills') -# Upload check env -UPLOAD_MAX_FILE_SIZE = int( - os.environ.get('UPLOAD_MAX_FILE_SIZE_MB', 100 * 1024)) * 1024 * 1024 -UPLOAD_SIZE_THRESHOLD_TO_ENFORCE_LFS = int( - os.environ.get('UPLOAD_SIZE_THRESHOLD_TO_ENFORCE_LFS', 1 * 1024 * 1024)) -UPLOAD_MAX_FILE_COUNT = int(os.environ.get('UPLOAD_MAX_FILE_COUNT', 100_000)) -UPLOAD_MAX_FILE_COUNT_IN_DIR = int( - os.environ.get('UPLOAD_MAX_FILE_COUNT_IN_DIR', 50_000)) -UPLOAD_NORMAL_FILE_SIZE_TOTAL_LIMIT = int( - os.environ.get('UPLOAD_NORMAL_FILE_SIZE_TOTAL_LIMIT', 500 * 1024 * 1024)) -UPLOAD_COMMIT_BATCH_SIZE = int(os.environ.get('UPLOAD_COMMIT_BATCH_SIZE', 256)) -UPLOAD_VALIDATE_BLOB_BATCH_SIZE = int( - os.environ.get('UPLOAD_VALIDATE_BLOB_BATCH_SIZE', 64)) -UPLOAD_ADAPTIVE_BATCH_SIZE = os.environ.get('UPLOAD_ADAPTIVE_BATCH_SIZE', - 'true').lower() == 'true' - -# ReAct progressive retry fallback -UPLOAD_REACT_ENABLED = os.environ.get('UPLOAD_REACT_ENABLED', - 'true').lower() == 'true' -UPLOAD_REACT_ROUND2_BASE_DELAY = int( - os.environ.get('UPLOAD_REACT_ROUND2_BASE_DELAY', 2)) -UPLOAD_REACT_ROUND3_FILE_DELAY = int( - os.environ.get('UPLOAD_REACT_ROUND3_FILE_DELAY', 30)) -UPLOAD_REACT_BACKOFF_MAX_EXPONENT = int( - os.environ.get('UPLOAD_REACT_BACKOFF_MAX_EXPONENT', 5)) -UPLOAD_REACT_MAX_DELAY = int(os.environ.get('UPLOAD_REACT_MAX_DELAY', 120)) - -UPLOAD_BLOB_TQDM_DISABLE_THRESHOLD = 5 * 1024 * 1024 -UPLOAD_USE_CACHE = os.environ.get('UPLOAD_USE_CACHE', 'true').lower() == 'true' - MODELSCOPE_ASCII = r""" _ .-') _ .-') _ ('-. .-') _ (`-. ('-. diff --git a/modelscope/hub/mcp_api.py b/modelscope/hub/mcp_api.py index 3613b92d..45503d15 100644 --- a/modelscope/hub/mcp_api.py +++ b/modelscope/hub/mcp_api.py @@ -113,40 +113,31 @@ class MCPApi(HubApi): if total_count is None or total_count < 1 or total_count > 100: raise ValueError('total_count must be between 1 and 100') - body = { - 'filter': filter or {}, - 'page_number': 1, - 'page_size': total_count, - 'search': search - } - try: - headers = self._build_bearer_headers( - token=token, token_required=False) - r = self.session.put( - url=self.mcp_base_url, headers=headers, json=body) - raise_for_http_status(r) - except requests.exceptions.RequestException as e: + api = self._api + if token: + from modelscope_hub.api import HubApi as _HubApi + api = _HubApi(token=token, endpoint=self.endpoint) + result = api.list_mcp_servers( + search=search, + page_number=1, + page_size=total_count, + filter=filter, + ) + except Exception as e: logger.error('Failed to get MCP servers: %s', e) raise MCPApiRequestError(f'Failed to get MCP servers: {e}') from e - try: - data = self._parse_openapi_response(r) - except RequestError as e: - raise MCPApiResponseError( - f'Invalid response from MCP servers list: {e}') from e - - mcp_server_list = data.get('mcp_server_list', []) mcp_config_list = [{ - 'name': item.get('name', ''), - 'id': item.get('id', ''), - 'description': item.get('description', '') - } for item in mcp_server_list] + 'name': + item.get('name') or item.get('Name') or '', + 'id': + item.get('id') or item.get('Id') or '', + 'description': + item.get('description') or item.get('Description') or '' + } for item in result.items] - return { - 'total_count': data.get('total_count', 0), - 'servers': mcp_config_list - } + return {'total_count': result.total_count, 'servers': mcp_config_list} def list_operational_mcp_servers(self, token: str = None) -> Dict[str, Any]: diff --git a/modelscope/hub/push_to_hub.py b/modelscope/hub/push_to_hub.py index df49ae5e..dc889e6a 100644 --- a/modelscope/hub/push_to_hub.py +++ b/modelscope/hub/push_to_hub.py @@ -9,6 +9,7 @@ from pathlib import Path from typing import List, Optional, Union import json +from modelscope_hub.compat.constants import get_upload_ignore_file_pattern from modelscope.hub.api import HubApi from modelscope.hub.constants import ModelVisibility @@ -128,7 +129,7 @@ def push_to_hub(repo_name, if token is None: token = os.environ.get('MODELSCOPE_API_TOKEN') if ignore_file_pattern is None: - ignore_file_pattern = os.environ.get('UPLOAD_IGNORE_FILE_PATTERN') + ignore_file_pattern = get_upload_ignore_file_pattern() assert repo_name is not None assert token is not None, 'Either pass in a token or to set `MODELSCOPE_API_TOKEN` in the environment variables.' assert os.path.isdir(output_dir) @@ -171,7 +172,7 @@ def push_to_hub_async(repo_name, if token is None: token = os.environ.get('MODELSCOPE_API_TOKEN') if ignore_file_pattern is None: - ignore_file_pattern = os.environ.get('UPLOAD_IGNORE_FILE_PATTERN') + ignore_file_pattern = get_upload_ignore_file_pattern() assert repo_name is not None assert token is not None, 'Either pass in a token or to set `MODELSCOPE_API_TOKEN` in the environment variables.' assert os.path.isdir(output_dir) diff --git a/modelscope/hub/utils/aigc.py b/modelscope/hub/utils/aigc.py index 59c53988..0ce1a707 100644 --- a/modelscope/hub/utils/aigc.py +++ b/modelscope/hub/utils/aigc.py @@ -1,10 +1,12 @@ # Copyright (c) Alibaba, Inc. and its affiliates. import glob import os -from typing import List, Optional +from typing import List, Optional, Tuple, Union from urllib.parse import urlparse import requests +from modelscope_hub.constants import (UPLOAD_BLOB_CONNECT_TIMEOUT_SECONDS, + UPLOAD_BLOB_READ_TIMEOUT_SECONDS) from tqdm.auto import tqdm from modelscope.hub.utils.utils import (MODELSCOPE_URL_SCHEME, @@ -83,6 +85,7 @@ class AigcModel: official_tags: Optional[List[str]] = None, model_source: Optional[str] = 'USER_UPLOAD', base_model_sub_type: Optional[str] = '', + readme_content: Optional[str] = None, ): """ Initializes the AigcModel helper. @@ -101,7 +104,12 @@ class AigcModel: model_source (str, optional): Source of the model. `USER_UPLOAD`, `TRAINED_FROM_MODELSCOPE` or `TRAINED_FROM_ALIYUN_FC`. Defaults to 'USER_UPLOAD'. base_model_sub_type (str, Optional): Sub vision foundation model type. Defaults to ''. e.g. `SD_1_5` + readme_content (str, optional): Complete README.md content. When creating an AIGC version, + it is committed to master before the immutable version snapshot is created. + Defaults to None. """ + if readme_content is not None and not isinstance(readme_content, str): + raise TypeError('readme_content must be a string or None') self.model_path = model_path self.aigc_type = aigc_type self.base_model_type = base_model_type @@ -109,6 +117,7 @@ class AigcModel: self.description = description self.model_source = model_source self.base_model_sub_type = base_model_sub_type + self.readme_content: Optional[str] = readme_content # Process cover images - convert local paths to base64 data URLs if cover_images is not None: processed_cover_images = [] @@ -310,7 +319,8 @@ class AigcModel: def preupload_weights(self, *, cookies: Optional[object] = None, - timeout: int = 300, + timeout: Optional[Union[int, Tuple[int, + int]]] = None, headers: Optional[dict] = None, endpoint: Optional[str] = None) -> None: """Pre-upload aigc model weights to the LFS server. @@ -320,15 +330,22 @@ class AigcModel: Args: cookies: Optional requests-style cookies (CookieJar/dict). If provided, preferred. - timeout: Request timeout seconds. + timeout: Optional requests timeout override. A scalar applies to + both connection and socket reads; a tuple is interpreted as + ``(connect timeout, read timeout)``. Defaults to the shared + modelscope-hub blob upload timeouts. headers: Optional headers. """ endpoint = endpoint or get_endpoint() + if timeout is None: + timeout = (UPLOAD_BLOB_CONNECT_TIMEOUT_SECONDS, + UPLOAD_BLOB_READ_TIMEOUT_SECONDS) endpoint_host: str = urlparse(endpoint.strip()).hostname.lstrip('www.') # https://lfs.modelscope.cn or https://pre-lfs.modelscope.cn + pre_endpoint_host = endpoint_host.lstrip('pre.') base_url: str = f'{MODELSCOPE_URL_SCHEME}lfs.{endpoint_host}' if not endpoint_host.startswith('pre') \ - else f'{MODELSCOPE_URL_SCHEME}pre-lfs.{endpoint_host.lstrip("pre.")}' + else f'{MODELSCOPE_URL_SCHEME}pre-lfs.{pre_endpoint_host}' url: str = f'{base_url}/api/v1/models/aigc/weights' @@ -395,6 +412,7 @@ class AigcModel: 'official_tags': self.official_tags, 'model_source': self.model_source, 'base_model_sub_type': self.base_model_sub_type, + 'readme_content': self.readme_content, } @classmethod diff --git a/requirements/hub.txt b/requirements/hub.txt index a564f367..6046f5fa 100644 --- a/requirements/hub.txt +++ b/requirements/hub.txt @@ -1,5 +1,5 @@ filelock -modelscope-hub>=0.3.0 +modelscope-hub>=0.3.1 packaging requests>=2.25 setuptools diff --git a/tests/hub/test_create_aigc_model.py b/tests/hub/test_create_aigc_model.py index 0529ebd6..ccaf027a 100644 --- a/tests/hub/test_create_aigc_model.py +++ b/tests/hub/test_create_aigc_model.py @@ -3,8 +3,9 @@ import os import tempfile import unittest import uuid +from unittest import mock -from requests.exceptions import HTTPError +import json from modelscope import HubApi from modelscope.hub.utils.aigc import AigcModel @@ -39,13 +40,8 @@ class TestCreateAigcModel(unittest.TestCase): delete_credential() @unittest.skipUnless(test_level() >= 2, 'skip test in current test level') - def test_create_aigc_model_expects_sha256_error(self): - """Test creating an AIGC model repository. - - This test is expected to fail with a 'sha256 not exits' error from the server. - This is the correct behavior when the server does not know the file yet. - This test verifies that the SDK is correctly forming and sending the request. - """ + def test_create_aigc_model(self): + """Test creating and uploading an AIGC model repository.""" logger.info(f'TEST: Attempting to create AIGC repo {self.repo_id} ...') # Login just before making the authenticated call. @@ -56,23 +52,132 @@ class TestCreateAigcModel(unittest.TestCase): model_path=self.tmp_file_path, aigc_type='Checkpoint', base_model_type='SD_XL', + readme_content='# AIGC integration test\n', ) - # 2. Attempt to create the model repository. - # We expect an HTTPError because the server API requires the file's sha256 - # to be known before creating the repo. - with self.assertRaises(HTTPError) as cm: - self.api.create_model( - model_id=self.repo_id, - aigc_model=aigc_model, + # 2. Create the repository through the AIGC-specific compatibility path. + model_url = self.api.create_model( + model_id=self.repo_id, + visibility=1, + aigc_model=aigc_model, + ) + + self.assertEqual(model_url, + f'{self.api.endpoint}/models/{self.repo_id}') + files = self.api.get_model_files(self.repo_id, revision='master') + paths = {item['Path'] for item in files} + self.assertIn(os.path.basename(self.tmp_file_path), paths) + readme_path = self.api.download_file( + self.repo_id, + repo_type='model', + file_path='README.md', + force=True, + ) + self.assertEqual( + readme_path.read_text(encoding='utf-8'), + '# AIGC integration test\n') + + +class TestAigcModelReadmeContent(unittest.TestCase): + + def setUp(self): + self.tmp_file = tempfile.NamedTemporaryFile( + suffix='.safetensors', delete=False) + self.tmp_file.write(b'dummy weights') + self.tmp_file.close() + + def tearDown(self): + os.remove(self.tmp_file.name) + + def test_optional_readme_content(self): + readme_content = '# AIGC v1.3\n\nCustom model card.\n' + aigc_model = AigcModel( + model_path=self.tmp_file.name, + aigc_type='LoRA', + base_model_type='SD_XL', + readme_content=readme_content, + ) + + self.assertEqual(aigc_model.readme_content, readme_content) + self.assertEqual(aigc_model.to_dict()['readme_content'], + readme_content) + + def test_readme_content_defaults_to_none(self): + aigc_model = AigcModel( + model_path=self.tmp_file.name, + aigc_type='LoRA', + base_model_type='SD_XL', + ) + + self.assertIsNone(aigc_model.readme_content) + + def test_from_json_file_accepts_readme_content(self): + config_file = tempfile.NamedTemporaryFile( + mode='w', suffix='.json', delete=False, encoding='utf-8') + self.addCleanup(os.remove, config_file.name) + readme_content = '# Loaded from JSON\n' + json.dump( + { + 'model_path': self.tmp_file.name, + 'aigc_type': 'LoRA', + 'base_model_type': 'SD_XL', + 'base_model_id': 'owner/base-model', + 'readme_content': readme_content, + }, config_file) + config_file.close() + + aigc_model = AigcModel.from_json_file(config_file.name) + + self.assertEqual(aigc_model.readme_content, readme_content) + + def test_readme_content_must_be_string(self): + with self.assertRaisesRegex(TypeError, + 'readme_content must be a string or None'): + AigcModel( + model_path=self.tmp_file.name, + aigc_type='LoRA', + base_model_type='SD_XL', + readme_content=123, ) - # Check if the error message is the one we expect. - # The actual error might be 'namespace is not valid' if run outside CI - # or 'sha256 not exits' if namespace is valid. Both are acceptable failures - # proving the SDK sent the request correctly. - error_str = str(cm.exception) - is_expected_error = 'sha256 not exits' in error_str or 'namespace' in error_str and 'is not valid' in error_str - self.assertTrue(is_expected_error, - f'Unexpected error message: {error_str}') - logger.info(f'TEST: Received expected error: {error_str}') + @mock.patch('modelscope.hub.utils.aigc.requests.put') + def test_preupload_uses_shared_blob_timeout_by_default(self, put): + put.return_value.json.return_value = {} + aigc_model = AigcModel( + model_path=self.tmp_file.name, + aigc_type='LoRA', + base_model_type='SD_XL', + ) + + aigc_model.preupload_weights( + cookies={'m_session_id': 'ms-test'}, headers={}) + + self.assertEqual(put.call_args.kwargs['timeout'], (30, 3600)) + + @mock.patch('modelscope.hub.utils.aigc.requests.put') + def test_preupload_preserves_explicit_timeout(self, put): + put.return_value.json.return_value = {} + aigc_model = AigcModel( + model_path=self.tmp_file.name, + aigc_type='LoRA', + base_model_type='SD_XL', + ) + + aigc_model.preupload_weights( + cookies={'m_session_id': 'ms-test'}, headers={}, timeout=17) + + self.assertEqual(put.call_args.kwargs['timeout'], 17) + + def test_legacy_upload_constants_match_modelscope_hub(self): + from modelscope.hub import constants as legacy + from modelscope_hub import constants as canonical + + self.assertEqual(legacy.UPLOAD_BLOB_TIMEOUT, + (canonical.UPLOAD_BLOB_CONNECT_TIMEOUT_SECONDS, + canonical.UPLOAD_BLOB_READ_TIMEOUT_SECONDS)) + self.assertEqual(legacy.UPLOAD_BLOB_MAX_RETRIES, + canonical.UPLOAD_BLOB_MAX_ATTEMPTS) + self.assertEqual(legacy.UPLOAD_SIZE_THRESHOLD_TO_ENFORCE_LFS, + canonical.UPLOAD_LFS_FORCE_THRESHOLD_BYTES) + self.assertEqual(legacy.UPLOAD_REACT_ROUND3_FILE_DELAY, + canonical.UPLOAD_RECOVERY_SINGLE_FILE_DELAY_SECONDS) diff --git a/tests/mcp/test_mcp_api.py b/tests/mcp/test_mcp_api.py index 08905b66..4b0291d0 100644 --- a/tests/mcp/test_mcp_api.py +++ b/tests/mcp/test_mcp_api.py @@ -1,5 +1,7 @@ # Copyright (c) Alibaba, Inc. and its affiliates. import unittest +from types import SimpleNamespace +from unittest import mock from modelscope.hub.mcp_api import MCPApi from modelscope.utils.logger import get_logger @@ -64,5 +66,41 @@ class MCPApiTest(unittest.TestCase): self.assertEqual(result['id'], '@modelcontextprotocol/fetch') +class MCPApiCompatTest(unittest.TestCase): + + def test_list_mcp_servers_delegates_to_modelscope_hub(self): + api = MCPApi() + delegated_api = mock.MagicMock() + delegated_api.list_mcp_servers.return_value = SimpleNamespace( + items=[{ + 'Name': 'Fetch', + 'Id': '@modelcontextprotocol/fetch', + 'Description': 'Fetch pages' + }], + total_count=1, + ) + + with mock.patch( + 'modelscope_hub.api.HubApi', + return_value=delegated_api) as hub_api_cls: + result = api.list_mcp_servers( + token='ms-readonly', + filter={'category': 'tools'}, + total_count=2, + search='fetch') + + hub_api_cls.assert_called_once_with( + token='ms-readonly', endpoint=api.endpoint) + delegated_api.list_mcp_servers.assert_called_once_with( + search='fetch', + page_number=1, + page_size=2, + filter={'category': 'tools'}, + ) + self.assertEqual(result['total_count'], 1) + self.assertEqual(result['servers'][0]['id'], + '@modelcontextprotocol/fetch') + + if __name__ == '__main__': unittest.main()