diff --git a/modelscope/hub/api.py b/modelscope/hub/api.py index e2454644..b34346d4 100644 --- a/modelscope/hub/api.py +++ b/modelscope/hub/api.py @@ -1011,25 +1011,11 @@ class HubApi: if owner_or_group: params['author'] = owner_or_group - cookies = self.get_cookies(access_token=token, cookies_required=False) - headers = self.builder_headers(self.headers) + headers = self._build_bearer_headers(token=token, token_required=False) - r = self.session.get( - path, - params=params, - cookies=cookies, - headers=headers - ) + r = self.session.get(path, params=params, headers=headers) raise_for_http_status(r) - resp = r.json() - - # OpenAPI success schema - if resp.get('success') is True and 'data' in resp: - return resp['data'] - else: - # Fallback for unexpected schema - msg = resp.get('message') or 'Failed to list datasets' - raise RequestError(msg) + return self._parse_openapi_response(r) def _check_cookie(self, use_cookies: Union[bool, CookieJar] = False) -> CookieJar: # noqa cookies = None @@ -1993,6 +1979,90 @@ class HubApi: return {MODELSCOPE_REQUEST_ID: str(uuid.uuid4().hex), **headers} + def _build_bearer_headers(self, + token: Optional[str] = None, + token_required: bool = False) -> Dict[str, str]: + """ + Build HTTP headers with optional Bearer token for OpenAPI endpoints. + + Token resolution order: + 1. Explicit token param + 2. self.token (set at construction) + 3. MODELSCOPE_API_TOKEN env var + 4. Locally cached cookies (m_session_id from login()) + + Args: + token: Optional access token for one-time authentication. + token_required: If True, raise ValueError when no token is available. + + Returns: + Headers dict with user-agent, request-id, and optionally Authorization. + + Raises: + ValueError: If token_required is True but no token is available. + """ + headers = self.builder_headers(self.headers) + + # Priority: explicit token > self.token > env var > local cookies + resolved_token = token or self.token or os.environ.get( + 'MODELSCOPE_API_TOKEN') + + # Fall back to locally cached cookies (m_session_id saved by login()) + if not resolved_token: + cookies = self.get_cookies() + if cookies: + for cookie in cookies: + if cookie.name == 'm_session_id': + resolved_token = cookie.value + break + + if resolved_token: + headers['Authorization'] = f'Bearer {resolved_token}' + elif token_required: + raise ValueError( + 'Authentication required but no token found. ' + 'You can pass the `token` argument, ' + 'or set MODELSCOPE_API_TOKEN environment variable, ' + 'or use HubApi(token=`your_sdk_token`). ' + 'Your token is available at https://modelscope.cn/my/myaccesstoken' + ) + return headers + + @staticmethod + def _parse_openapi_response(response: 'requests.Response') -> Dict[str, Any]: + """ + Parse OpenAPI response with unified JSON parsing and data extraction. + + Handles the standard OpenAPI response envelope: + {"success": bool, "data": {...}, "message": str} + Also handles the simpler envelope where only "data" is present. + + Args: + response: requests Response object (HTTP status already validated). + + Returns: + Parsed 'data' dict from the response envelope. + + Raises: + RequestError: If JSON parsing fails or business-level error is returned. + """ + try: + resp = response.json() + except (requests.exceptions.JSONDecodeError, ValueError) as e: + logger.error(f'JSON parsing failed: {e}') + raise RequestError(f'Invalid JSON response: {e}') from e + + # OpenAPI envelope with explicit success field + if isinstance(resp, dict) and 'success' in resp: + if resp.get('success') is True and 'data' in resp: + return resp['data'] + else: + msg = resp.get('message') or 'OpenAPI request failed' + raise RequestError(msg) + + # Simple envelope with data field only (e.g., MCP API) + return resp.get('data', {}) if isinstance(resp, dict) else {} + def get_file_base_path(self, repo_id: str, endpoint: Optional[str] = None) -> str: _namespace, _dataset_name = repo_id.split('/') if not endpoint: diff --git a/modelscope/hub/mcp_api.py b/modelscope/hub/mcp_api.py index 40aa07e6..3613b92d 100644 --- a/modelscope/hub/mcp_api.py +++ b/modelscope/hub/mcp_api.py @@ -10,7 +10,7 @@ from typing import Any, Dict, Optional import requests from modelscope.hub.api import HubApi -from modelscope.hub.errors import raise_for_http_status +from modelscope.hub.errors import RequestError, raise_for_http_status from modelscope.utils.logger import get_logger # Configure logging @@ -46,40 +46,20 @@ class MCPApi(HubApi): Different methods have different token requirements - see individual method docs. """ - def __init__(self, endpoint: Optional[str] = None) -> None: + def __init__(self, + endpoint: Optional[str] = None, + token: Optional[str] = None) -> None: """ Initialize MCP API. Args: endpoint: The modelscope server address. Defaults to None (uses default endpoint). + token: Optional access token for Bearer authentication. """ - super().__init__(endpoint=endpoint) + super().__init__(endpoint=endpoint, token=token) self.mcp_base_url = self.endpoint + MCP_API_PATH - @staticmethod - def _handle_response(r: requests.Response) -> Dict[str, Any]: - """ - Handle HTTP response with unified error handling and JSON parsing. - - Args: - r: requests Response object - - Returns: - Parsed response data dict - - Raises: - MCPApiResponseError: If JSON parsing fails - """ - try: - resp = r.json() - except requests.exceptions.JSONDecodeError as e: - logger.error(f'JSON parsing failed: {e}') - logger.error(f'Response content: {r.text}') - raise MCPApiResponseError(f'Invalid JSON response: {e}') from e - - return resp.get('data', {}) - @staticmethod def _get_server_name_from_id(server_id: str) -> str: """Extract server name from server ID.""" @@ -141,18 +121,21 @@ class MCPApi(HubApi): } try: - cookies = self.get_cookies(token) + headers = self._build_bearer_headers( + token=token, token_required=False) r = self.session.put( - url=self.mcp_base_url, - headers=self.builder_headers(self.headers), - json=body, - cookies=cookies) + url=self.mcp_base_url, headers=headers, json=body) raise_for_http_status(r) except requests.exceptions.RequestException as e: logger.error('Failed to get MCP servers: %s', e) raise MCPApiRequestError(f'Failed to get MCP servers: {e}') from e - data = self._handle_response(r) + 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', ''), @@ -204,12 +187,11 @@ class MCPApi(HubApi): } """ url = f'{self.mcp_base_url}/operational' - headers = self.builder_headers(self.headers) try: - cookies = self.get_cookies( - access_token=token, cookies_required=True) - r = self.session.get(url, headers=headers, cookies=cookies) + headers = self._build_bearer_headers( + token=token, token_required=True) + r = self.session.get(url, headers=headers) raise_for_http_status(r) except requests.exceptions.RequestException as e: logger.error(f'Failed to get operational MCP servers: {e}') @@ -218,7 +200,12 @@ class MCPApi(HubApi): logger.debug(f'Response status code: {r.status_code}') - data = self._handle_response(r) + try: + data = self._parse_openapi_response(r) + except RequestError as e: + raise MCPApiResponseError( + f'Invalid response from operational MCP servers: {e}') from e + mcp_server_list = data.get('mcp_server_list', []) mcp_config_list = [] @@ -230,8 +217,7 @@ class MCPApi(HubApi): mcp_config['mcp_servers'] = [] for operational_url in item.get('operational_urls', []): mcp_config['mcp_servers'].append({ - 'type': - operational_url.get('url').split('/')[-1], + 'type': (operational_url.get('url') or '').split('/')[-1], 'url': operational_url.get('url', '') }) @@ -286,23 +272,23 @@ class MCPApi(HubApi): raise ValueError('server_id cannot be empty') url = f'{self.mcp_base_url}/{server_id}' - headers = self.builder_headers(self.headers) try: - cookies = self.get_cookies(token) + headers = self._build_bearer_headers( + token=token, token_required=False) r = self.session.get( - url, - headers=headers, - params={'get_operational_url': - True}, # Always get operational URLs - cookies=cookies) + url, headers=headers, params={'get_operational_url': True}) raise_for_http_status(r) except requests.exceptions.RequestException as e: logger.error(f'Failed to get MCP server {server_id}: {e}') raise MCPApiRequestError( f'Failed to get MCP server {server_id}: {e}') from e - data = self._handle_response(r) + try: + data = self._parse_openapi_response(r) + except RequestError as e: + raise MCPApiResponseError( + f'Invalid response from MCP server {server_id}: {e}') from e result = { 'name': data.get('name', ''), @@ -318,7 +304,7 @@ class MCPApi(HubApi): if server_name and operational_urls: for operational_url in operational_urls: mcp_config = { - 'type': operational_url.get('url').split('/')[-1], + 'type': (operational_url.get('url') or '').split('/')[-1], 'url': operational_url.get('url', '') } mcp_config_list.append(mcp_config) diff --git a/tests/mcp/test_mcp_api.py b/tests/mcp/test_mcp_api.py index cce8cc82..08905b66 100644 --- a/tests/mcp/test_mcp_api.py +++ b/tests/mcp/test_mcp_api.py @@ -15,7 +15,7 @@ class MCPApiTest(unittest.TestCase): self.api = MCPApi() self.api.login(TEST_ACCESS_TOKEN1) - @unittest.skipUnless(test_level() >= 2, 'skip test in current test level') + @unittest.skipUnless(test_level() >= 0, 'skip test in current test level') def test_list_mcp_servers(self): """Test list_mcp_servers functionality and validation.""" result = self.api.list_mcp_servers(total_count=5) @@ -31,7 +31,7 @@ class MCPApiTest(unittest.TestCase): for field in ['name', 'id', 'description']: self.assertIn(field, server) - @unittest.skipUnless(test_level() >= 2, 'skip test in current test level') + @unittest.skipUnless(test_level() >= 0, 'skip test in current test level') def test_list_operational_mcp_servers(self): """Test list_operational_mcp_servers functionality.""" result = self.api.list_operational_mcp_servers() @@ -53,7 +53,7 @@ class MCPApiTest(unittest.TestCase): self.assertIn('url', first_config) self.assertTrue(first_config['url'].startswith('https://')) - @unittest.skipUnless(test_level() >= 2, 'skip test in current test level') + @unittest.skipUnless(test_level() >= 0, 'skip test in current test level') def test_get_mcp_server(self): """Test get_mcp_server functionality and validation.""" result = self.api.get_mcp_server('@modelcontextprotocol/fetch')