mirror of
https://github.com/modelscope/modelscope.git
synced 2026-09-01 19:49:03 +02:00
[Fix]dynamic module auto map double dash (#1761)
* fix: resolve cross-repo auto_map when cache paths contain --
modelscope_hub 0.1.x layout embeds -- in local dirs; rejoining that
path into class_reference broke transformers' split("--"). Pass the
local snapshot as pretrained_model_name_or_path with a bare class name
instead, and stop mutating the args tuple in place.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix: reject empty model name in trusted owner cache paths
check_model_from_owner_group treated paths like iic--/snapshots/v1 as
trusted because split('--') still yields two parts. Require both owner
and name segments to be non-empty.
Co-authored-by: Cursor <cursoragent@cursor.com>
* test: cover remote pretrained args tuple rebuild in dynamic module patcher
The TypeError from mutating *args was already fixed with the auto_map
double-dash change; add an explicit regression test for that branch.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix: derive valid module names from snapshot revision paths
Path(model_dir).stem on revisions like v1.0.4 produced v1.0, so
importlib treated it as package v1 and raised ModuleNotFoundError.
Use the owner--name (+ revision) segment and sanitize to an identifier.
Co-authored-by: Cursor <cursoragent@cursor.com>
* test: cover module naming for dotted snapshot revisions
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix: handle pretrained_model_name_or_path passed via kwargs
When the arg is only in kwargs, downloading and then forcing a
positional overwrite caused a duplicate-keyword TypeError. Resolve and
update kwargs or args consistently for both the download and cross-repo
branches.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix lint
* fix(docker): catch JSONDecodeError when querying Docker Hub tags
Non-JSON Hub responses (block pages, 5xx HTML) would bypass URLError
handling and crash json.load; surface them as RuntimeError instead.
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: suluyan <suluyan.sly@aliabab-inc.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -1,5 +1,4 @@
|
||||
import argparse
|
||||
import json
|
||||
import os
|
||||
import platform
|
||||
import re
|
||||
@@ -10,6 +9,8 @@ from copy import copy
|
||||
from datetime import datetime
|
||||
from typing import Any, List, Optional
|
||||
|
||||
import json
|
||||
|
||||
docker_registry = os.environ['DOCKER_REGISTRY']
|
||||
assert docker_registry, 'You must pass a valid DOCKER_REGISTRY'
|
||||
timestamp = datetime.now()
|
||||
@@ -507,16 +508,15 @@ class AmdImageBuilder(Builder):
|
||||
@classmethod
|
||||
def _fetch_rocm_tags(cls, page_size: int = 100) -> List[dict]:
|
||||
tags: List[dict] = []
|
||||
url = (
|
||||
f'https://hub.docker.com/v2/repositories/{VLLM_ROCM_REPO}/tags'
|
||||
f'?page_size={page_size}&ordering=-last_updated')
|
||||
url = (f'https://hub.docker.com/v2/repositories/{VLLM_ROCM_REPO}/tags'
|
||||
f'?page_size={page_size}&ordering=-last_updated')
|
||||
while url:
|
||||
req = urllib.request.Request(
|
||||
url, headers={'User-Agent': 'modelscope-docker-builder'})
|
||||
try:
|
||||
with urllib.request.urlopen(req, timeout=60) as resp:
|
||||
payload = json.load(resp)
|
||||
except urllib.error.URLError as exc:
|
||||
except (urllib.error.URLError, json.JSONDecodeError) as exc:
|
||||
raise RuntimeError(
|
||||
f'Failed to query Docker Hub tags for {VLLM_ROCM_REPO}: '
|
||||
f'{exc}') from exc
|
||||
@@ -569,8 +569,8 @@ class AmdImageBuilder(Builder):
|
||||
def init_args(self, args: Any) -> Any:
|
||||
# Auto-discover from Docker Hub unless an explicit override is given.
|
||||
override = getattr(args, 'base_image_tag', None)
|
||||
if override and str(override).strip() and str(override).strip().lower(
|
||||
) not in {'auto', 'latest'}:
|
||||
if override and str(override).strip() and str(
|
||||
override).strip().lower() not in {'auto', 'latest'}:
|
||||
args.base_image_tag = str(override).strip()
|
||||
if not self._is_specific_release_tag(args.base_image_tag):
|
||||
raise ValueError(
|
||||
@@ -630,9 +630,9 @@ class AmdImageBuilder(Builder):
|
||||
' if os.environ.get(k): info[k.lower()]=os.environ[k]\n'
|
||||
'print(json.dumps(info))\n')
|
||||
for py in ('python3', 'python'):
|
||||
result = cls._run_capture(
|
||||
'docker', 'run', '--rm', '--network', 'none', '--entrypoint',
|
||||
py, base_image, '-c', script)
|
||||
result = cls._run_capture('docker', 'run', '--rm', '--network',
|
||||
'none', '--entrypoint', py, base_image,
|
||||
'-c', script)
|
||||
if result.returncode == 0 and result.stdout.strip():
|
||||
try:
|
||||
return json.loads(result.stdout.strip().splitlines()[-1])
|
||||
@@ -714,9 +714,11 @@ class AmdImageBuilder(Builder):
|
||||
|
||||
versions = {
|
||||
'rocm': cls._normalize_version(rocm) if rocm else None,
|
||||
'python': cls._normalize_version(python_ver) if python_ver else None,
|
||||
'python':
|
||||
cls._normalize_version(python_ver) if python_ver else None,
|
||||
'torch': cls._normalize_version(torch_ver) if torch_ver else None,
|
||||
'ubuntu': cls._normalize_version(ubuntu_ver) if ubuntu_ver else None,
|
||||
'ubuntu':
|
||||
cls._normalize_version(ubuntu_ver) if ubuntu_ver else None,
|
||||
}
|
||||
print('Probed AMD base image versions:')
|
||||
for key, value in versions.items():
|
||||
@@ -744,9 +746,8 @@ class AmdImageBuilder(Builder):
|
||||
raise RuntimeError(
|
||||
'AMD image tag requires probed rocm/python/torch versions. '
|
||||
f'Got rocm={rocm}, python={py_tag}, torch={torch}')
|
||||
return (
|
||||
f'{docker_registry}:ubuntu{ubuntu}-rocm{rocm}-{py_tag}-'
|
||||
f'torch{torch}-{self.args.modelscope_version}-test')
|
||||
return (f'{docker_registry}:ubuntu{ubuntu}-rocm{rocm}-{py_tag}-'
|
||||
f'torch{torch}-{self.args.modelscope_version}-test')
|
||||
|
||||
def _log_base_image_info(self) -> int:
|
||||
base_image = self.args.base_image
|
||||
@@ -767,8 +768,8 @@ class AmdImageBuilder(Builder):
|
||||
print(f'AMD base image inspect warning: {result.stderr.strip()}')
|
||||
|
||||
versions = self.probe_base_image_versions(base_image)
|
||||
if not versions.get('rocm') or not versions.get('python') or not versions.get(
|
||||
'torch'):
|
||||
if not versions.get('rocm') or not versions.get(
|
||||
'python') or not versions.get('torch'):
|
||||
print('ERROR: failed to probe rocm/python/torch from base image')
|
||||
return 1
|
||||
self.args.amd_rocm_version = versions['rocm']
|
||||
@@ -779,9 +780,7 @@ class AmdImageBuilder(Builder):
|
||||
self.args.amd_ubuntu_version = versions['ubuntu']
|
||||
else:
|
||||
self.args.amd_ubuntu_version = self.args.ubuntu_version
|
||||
print(
|
||||
f'AMD output image tag will be: {self.image()}'
|
||||
)
|
||||
print(f'AMD output image tag will be: {self.image()}')
|
||||
print('=' * 60)
|
||||
return 0
|
||||
|
||||
@@ -801,10 +800,9 @@ class AmdImageBuilder(Builder):
|
||||
rocm = self.args.amd_rocm_version
|
||||
py_tag = self.args.amd_python_tag
|
||||
torch = self.args.amd_torch_version
|
||||
image_tag2 = (
|
||||
f'{docker_registry}:ubuntu{ubuntu}-rocm{rocm}-{py_tag}-'
|
||||
f'torch{torch}-{self.args.modelscope_version}-'
|
||||
f'{formatted_time}-test')
|
||||
image_tag2 = (f'{docker_registry}:ubuntu{ubuntu}-rocm{rocm}-{py_tag}-'
|
||||
f'torch{torch}-{self.args.modelscope_version}-'
|
||||
f'{formatted_time}-test')
|
||||
ret = self.run_cmd('docker', 'tag', image_name, image_tag2)
|
||||
if ret != 0:
|
||||
return ret
|
||||
|
||||
@@ -147,12 +147,13 @@ def check_model_from_owner_group(model_dir: str,
|
||||
if group in owner_group:
|
||||
return True
|
||||
# Also check cache path pattern: {cache_root}/{owner}--{model_name}/snapshots/{revision}
|
||||
# Require exactly "{owner}--{name}" format (2 segments split by --)
|
||||
# to prevent spoofing via accounts like "iic--hacked" which would
|
||||
# produce paths like "iic--hacked--evil" and bypass the check.
|
||||
# Require exactly "{owner}--{name}" with both segments non-empty
|
||||
# to prevent spoofing via accounts like "iic--hacked" (paths like
|
||||
# "iic--hacked--evil") or empty names like "iic--".
|
||||
grandparent = os.path.basename(os.path.dirname(parent_dir))
|
||||
if '--' in grandparent:
|
||||
parts = grandparent.split('--')
|
||||
if len(parts) == 2 and parts[0] in owner_group:
|
||||
# Both owner and name must be non-empty; reject "iic--" / "--name".
|
||||
if len(parts) == 2 and all(parts) and parts[0] in owner_group:
|
||||
return True
|
||||
return False
|
||||
|
||||
@@ -161,16 +161,43 @@ def _get_class_from_dynamic_module(class_reference, *args, **kwargs):
|
||||
When a config's ``auto_map`` references another repo, transformers calls
|
||||
``get_class_from_dynamic_module`` to fetch it. This wrapper ensures that
|
||||
fetch goes through ModelScope instead of HuggingFace.
|
||||
|
||||
Cross-repo ``auto_map`` entries use ``repo_id--module.Class``. After
|
||||
``snapshot_download``, the local cache path may itself contain ``--``
|
||||
(modelscope_hub 0.1.x layout: ``models/{owner}--{name}/snapshots/...``).
|
||||
Re-joining that path with ``--`` would make transformers'
|
||||
``class_reference.split("--")`` raise ``ValueError``. Instead, pass the
|
||||
local directory as ``pretrained_model_name_or_path`` and the bare
|
||||
``module.Class`` as ``class_reference`` so transformers takes the
|
||||
``os.path.isdir`` branch.
|
||||
"""
|
||||
from transformers.dynamic_module_utils import origin_get_class_from_dynamic_module
|
||||
if 'pretrained_model_name_or_path' in inspect.signature(
|
||||
origin_get_class_from_dynamic_module).parameters:
|
||||
pretrained_model_name_or_path = args[0]
|
||||
if not os.path.exists(pretrained_model_name_or_path):
|
||||
from modelscope import snapshot_download
|
||||
args[0] = snapshot_download(pretrained_model_name_or_path)
|
||||
has_pretrained_arg = (
|
||||
'pretrained_model_name_or_path'
|
||||
in inspect.signature(origin_get_class_from_dynamic_module).parameters)
|
||||
# Resolve pretrained_model_name_or_path from kwargs or positional args.
|
||||
# ``args`` is a tuple; never mutate it in place.
|
||||
pretrained_in_kwargs = False
|
||||
pretrained_model_name_or_path = None
|
||||
if has_pretrained_arg:
|
||||
if 'pretrained_model_name_or_path' in kwargs:
|
||||
pretrained_model_name_or_path = kwargs[
|
||||
'pretrained_model_name_or_path']
|
||||
pretrained_in_kwargs = True
|
||||
elif args:
|
||||
pretrained_model_name_or_path = args[0]
|
||||
if (pretrained_model_name_or_path is not None
|
||||
and not os.path.exists(pretrained_model_name_or_path)):
|
||||
from modelscope import snapshot_download
|
||||
downloaded_path = snapshot_download(pretrained_model_name_or_path)
|
||||
if pretrained_in_kwargs:
|
||||
kwargs['pretrained_model_name_or_path'] = downloaded_path
|
||||
else:
|
||||
args = (downloaded_path, ) + args[1:]
|
||||
pretrained_model_name_or_path = downloaded_path
|
||||
if '--' in class_reference:
|
||||
repo_id, class_reference = class_reference.split('--')
|
||||
# Only the first ``--`` is the auto_map delimiter (repo vs module).
|
||||
repo_id, class_reference = class_reference.split('--', 1)
|
||||
if not os.path.exists(repo_id):
|
||||
download_kwargs = {}
|
||||
extra_allow_file_pattern = _decide_allow_file_pattern(
|
||||
@@ -182,7 +209,18 @@ def _get_class_from_dynamic_module(class_reference, *args, **kwargs):
|
||||
download_kwargs['ignore_file_pattern'] = ignore_file_pattern
|
||||
from modelscope import snapshot_download
|
||||
repo_id = snapshot_download(repo_id, **download_kwargs)
|
||||
class_reference = repo_id + '--' + class_reference
|
||||
if has_pretrained_arg:
|
||||
# Local path + bare class name; do not rejoin with ``--``.
|
||||
# Keep kwargs/positional form consistent with the original call.
|
||||
if pretrained_in_kwargs:
|
||||
kwargs['pretrained_model_name_or_path'] = repo_id
|
||||
else:
|
||||
args = (repo_id, ) + args[1:]
|
||||
else:
|
||||
# Legacy transformers without pretrained_model_name_or_path.
|
||||
# Unsafe if repo_id (local cache) contains '--'; modern
|
||||
# transformers always take the branch above.
|
||||
class_reference = repo_id + '--' + class_reference
|
||||
return origin_get_class_from_dynamic_module(class_reference, *args,
|
||||
**kwargs)
|
||||
|
||||
|
||||
@@ -374,6 +374,30 @@ def create_module_from_files(file_list, file_prefix, module_name):
|
||||
importlib.invalidate_caches()
|
||||
|
||||
|
||||
def _module_name_from_model_dir(model_dir: str) -> str:
|
||||
"""Derive a valid Python package name from a local model directory.
|
||||
|
||||
modelscope_hub 0.1.x returns paths like
|
||||
``{cache}/models/{owner}--{name}/snapshots/{revision}``. Using
|
||||
``Path(model_dir).stem`` on a revision such as ``v1.0.4`` yields
|
||||
``v1.0``, and ``importlib.import_module('v1.0.xxx')`` fails with
|
||||
``ModuleNotFoundError: No module named 'v1'`` because dots are package
|
||||
separators. Prefer the ``{owner}--{name}`` segment (plus revision for
|
||||
isolation) and sanitize to a valid identifier.
|
||||
"""
|
||||
path = Path(model_dir).resolve()
|
||||
if path.parent.name == 'snapshots':
|
||||
base = f'{path.parent.parent.name}__{path.name}'
|
||||
else:
|
||||
# Use the full directory name, not .stem, so dotted names are kept
|
||||
# intact before sanitization (stem would turn ``v1.0.4`` into ``v1.0``).
|
||||
base = path.name
|
||||
module_name = re.sub(r'[^0-9A-Za-z_]', '_', base)
|
||||
if not module_name or module_name[0].isdigit():
|
||||
module_name = f'm_{module_name}'
|
||||
return module_name
|
||||
|
||||
|
||||
def import_module_from_model_dir(model_dir):
|
||||
""" import all the necessary module from a model dir
|
||||
|
||||
@@ -383,7 +407,6 @@ def import_module_from_model_dir(model_dir):
|
||||
No returns, raise error if failed
|
||||
|
||||
"""
|
||||
from pathlib import Path
|
||||
file_scanner = FilesAstScanning()
|
||||
file_scanner.traversal_files(model_dir, include_init=True)
|
||||
file_dirs = file_scanner.file_dirs
|
||||
@@ -395,7 +418,7 @@ def import_module_from_model_dir(model_dir):
|
||||
if BASE_MODULE_DIR not in sys.path:
|
||||
sys.path.append(BASE_MODULE_DIR)
|
||||
|
||||
module_name = Path(model_dir).stem
|
||||
module_name = _module_name_from_model_dir(model_dir)
|
||||
|
||||
# in order to keep forward compatibility, we add module path to
|
||||
# sys.path so that submodule can be imported directly as before
|
||||
|
||||
@@ -44,15 +44,12 @@ class TestStreamLoad(unittest.TestCase):
|
||||
pass
|
||||
|
||||
self.assertIs(HfFileSystem._open, hf_datasets_util._hf_fs_open)
|
||||
self.assertIsNot(
|
||||
hf_datasets_util._hf_fs_open_original,
|
||||
hf_datasets_util._hf_fs_open)
|
||||
self.assertIs(
|
||||
HfFileSystem.__init__,
|
||||
hf_datasets_util._hf_fs_init_with_cookie)
|
||||
self.assertIsNot(
|
||||
hf_datasets_util._hf_fs_init_original,
|
||||
hf_datasets_util._hf_fs_init_with_cookie)
|
||||
self.assertIsNot(hf_datasets_util._hf_fs_open_original,
|
||||
hf_datasets_util._hf_fs_open)
|
||||
self.assertIs(HfFileSystem.__init__,
|
||||
hf_datasets_util._hf_fs_init_with_cookie)
|
||||
self.assertIsNot(hf_datasets_util._hf_fs_init_original,
|
||||
hf_datasets_util._hf_fs_init_with_cookie)
|
||||
finally:
|
||||
HfFileSystem._open = hf_fs_open_before
|
||||
HfFileSystem.__init__ = hf_fs_init_before
|
||||
@@ -75,7 +72,8 @@ class TestStreamLoad(unittest.TestCase):
|
||||
'load_dataset',
|
||||
side_effect=RuntimeError('load failed')):
|
||||
with self.assertRaises(RuntimeError):
|
||||
with hf_datasets_util.load_dataset_with_ctx(streaming=True):
|
||||
with hf_datasets_util.load_dataset_with_ctx(
|
||||
streaming=True):
|
||||
pass
|
||||
|
||||
self.assertIs(HfFileSystem._open, hf_fs_open_clean)
|
||||
|
||||
@@ -249,19 +249,182 @@ class HFUtilTest(unittest.TestCase):
|
||||
f'Expected no weight files in {model_dir}, but found: '
|
||||
f"{[f for f in files if f.endswith('.safetensors') or f.endswith('.bin')]}"
|
||||
)
|
||||
cache_dir = os.path.dirname(model_dir)
|
||||
cache_dir = os.path.dirname(cache_dir)
|
||||
model_dir_2 = os.path.join(cache_dir, 'nomic-ai', 'nomic-bert-2048')
|
||||
if os.path.exists(model_dir_2):
|
||||
files = os.listdir(model_dir_2)
|
||||
has_weight_files = any(
|
||||
f.endswith('.safetensors') or f.endswith('.bin')
|
||||
for f in files)
|
||||
self.assertFalse(
|
||||
has_weight_files,
|
||||
f'Expected no weight files in {model_dir}, but found: '
|
||||
f"{[f for f in files if f.endswith('.safetensors') or f.endswith('.bin')]}"
|
||||
)
|
||||
# modelscope_hub 0.1.x layout uses models/{owner}--{name}/...;
|
||||
# older layout used {owner}/{name}/. Accept either.
|
||||
cache_root = model_dir
|
||||
for _ in range(4):
|
||||
parent = os.path.dirname(cache_root)
|
||||
if parent == cache_root:
|
||||
break
|
||||
cache_root = parent
|
||||
candidates = [
|
||||
os.path.join(cache_root, 'nomic-ai', 'nomic-bert-2048'),
|
||||
os.path.join(cache_root, 'models',
|
||||
'nomic-ai--nomic-bert-2048'),
|
||||
]
|
||||
for model_dir_2 in candidates:
|
||||
if not os.path.exists(model_dir_2):
|
||||
continue
|
||||
# Walk into snapshots/{rev} if present.
|
||||
check_dirs = [model_dir_2]
|
||||
snapshots = os.path.join(model_dir_2, 'snapshots')
|
||||
if os.path.isdir(snapshots):
|
||||
check_dirs.extend(
|
||||
os.path.join(snapshots, d)
|
||||
for d in os.listdir(snapshots)
|
||||
if os.path.isdir(os.path.join(snapshots, d)))
|
||||
for check_dir in check_dirs:
|
||||
files = os.listdir(check_dir)
|
||||
has_weight_files = any(
|
||||
f.endswith('.safetensors') or f.endswith('.bin')
|
||||
for f in files)
|
||||
self.assertFalse(
|
||||
has_weight_files,
|
||||
f'Expected no weight files in {check_dir}, but found: '
|
||||
f"{[f for f in files if f.endswith('.safetensors') or f.endswith('.bin')]}"
|
||||
)
|
||||
|
||||
def test_dynamic_module_double_dash_cache_path(self):
|
||||
"""Cross-repo auto_map must survive cache paths that contain '--'.
|
||||
|
||||
modelscope_hub 0.1.x stores repos under ``models/{owner}--{name}/``.
|
||||
Rejoining that path into ``class_reference`` with ``--`` makes
|
||||
transformers' ``split("--")`` raise ValueError.
|
||||
"""
|
||||
from unittest import mock
|
||||
|
||||
from modelscope.utils.hf_util.patcher import \
|
||||
_get_class_from_dynamic_module
|
||||
|
||||
tmp = tempfile.mkdtemp()
|
||||
self.addCleanup(shutil.rmtree, tmp, ignore_errors=True)
|
||||
local_path = os.path.join(tmp, 'models', 'nomic-ai--nomic-bert-2048',
|
||||
'snapshots', 'rev')
|
||||
os.makedirs(local_path)
|
||||
pretrained = os.path.join(tmp, 'models',
|
||||
'nomic-ai--nomic-embed-text-v1.5',
|
||||
'snapshots', 'rev')
|
||||
os.makedirs(pretrained)
|
||||
|
||||
captured = {}
|
||||
|
||||
def fake_origin(class_reference, pretrained_model_name_or_path, *args,
|
||||
**kwargs):
|
||||
# Signature must match transformers so has_pretrained_arg is True.
|
||||
captured['class_reference'] = class_reference
|
||||
captured['pretrained'] = pretrained_model_name_or_path
|
||||
return type('DummyConfig', (), {})
|
||||
|
||||
class_ref = ('nomic-ai/nomic-bert-2048--'
|
||||
'configuration_hf_nomic_bert.NomicBertConfig')
|
||||
|
||||
# create=True: do not permanently leave origin_* on the module
|
||||
# (would break test_import_not_pollute_dynamic_module).
|
||||
with mock.patch(
|
||||
'transformers.dynamic_module_utils.origin_get_class_from_dynamic_module',
|
||||
new=fake_origin,
|
||||
create=True):
|
||||
with mock.patch(
|
||||
'modelscope.snapshot_download', return_value=local_path):
|
||||
_get_class_from_dynamic_module(class_ref, pretrained)
|
||||
|
||||
# Must pass bare module.Class (no '--') so transformers does not split.
|
||||
self.assertEqual(captured['class_reference'],
|
||||
'configuration_hf_nomic_bert.NomicBertConfig')
|
||||
# Local cache path (which contains '--') is pretrained_model_name_or_path.
|
||||
self.assertEqual(captured['pretrained'], local_path)
|
||||
|
||||
def test_dynamic_module_remote_pretrained_tuple_args(self):
|
||||
"""Remote pretrained_model_name_or_path must not mutate args in place.
|
||||
|
||||
``*args`` is a tuple; ``args[0] = snapshot_download(...)`` raises
|
||||
TypeError. Rebuild the tuple instead (regression from 9379504f).
|
||||
"""
|
||||
from unittest import mock
|
||||
|
||||
from modelscope.utils.hf_util.patcher import \
|
||||
_get_class_from_dynamic_module
|
||||
|
||||
tmp = tempfile.mkdtemp()
|
||||
self.addCleanup(shutil.rmtree, tmp, ignore_errors=True)
|
||||
downloaded = os.path.join(tmp, 'models', 'org--model', 'snapshots',
|
||||
'rev')
|
||||
os.makedirs(downloaded)
|
||||
|
||||
captured = {}
|
||||
|
||||
def fake_origin(class_reference, pretrained_model_name_or_path, *args,
|
||||
**kwargs):
|
||||
captured['class_reference'] = class_reference
|
||||
captured['pretrained'] = pretrained_model_name_or_path
|
||||
return type('DummyConfig', (), {})
|
||||
|
||||
# No '--' in class_reference: only the pretrained download branch runs.
|
||||
remote_id = 'org/model-not-on-disk'
|
||||
with mock.patch(
|
||||
'transformers.dynamic_module_utils.origin_get_class_from_dynamic_module',
|
||||
new=fake_origin,
|
||||
create=True):
|
||||
with mock.patch(
|
||||
'modelscope.snapshot_download',
|
||||
return_value=downloaded) as sd:
|
||||
# Must not raise TypeError: 'tuple' object does not support
|
||||
# item assignment.
|
||||
_get_class_from_dynamic_module('modeling.Foo', remote_id)
|
||||
|
||||
sd.assert_called_once_with(remote_id)
|
||||
self.assertEqual(captured['class_reference'], 'modeling.Foo')
|
||||
self.assertEqual(captured['pretrained'], downloaded)
|
||||
|
||||
def test_dynamic_module_pretrained_via_kwargs(self):
|
||||
"""pretrained_model_name_or_path may be passed as a keyword argument."""
|
||||
from unittest import mock
|
||||
|
||||
from modelscope.utils.hf_util.patcher import \
|
||||
_get_class_from_dynamic_module
|
||||
|
||||
tmp = tempfile.mkdtemp()
|
||||
self.addCleanup(shutil.rmtree, tmp, ignore_errors=True)
|
||||
downloaded = os.path.join(tmp, 'models', 'org--model', 'snapshots',
|
||||
'rev')
|
||||
cross_repo = os.path.join(tmp, 'models', 'org--other', 'snapshots',
|
||||
'rev')
|
||||
os.makedirs(downloaded)
|
||||
os.makedirs(cross_repo)
|
||||
|
||||
captured = {}
|
||||
|
||||
def fake_origin(class_reference, pretrained_model_name_or_path, *args,
|
||||
**kwargs):
|
||||
captured['class_reference'] = class_reference
|
||||
captured['pretrained'] = pretrained_model_name_or_path
|
||||
captured['kwargs'] = kwargs
|
||||
return type('DummyConfig', (), {})
|
||||
|
||||
remote_id = 'org/model-not-on-disk'
|
||||
class_ref = 'org/other--configuration_foo.FooConfig'
|
||||
|
||||
def fake_download(repo_id, **kwargs):
|
||||
if repo_id == remote_id:
|
||||
return downloaded
|
||||
if repo_id == 'org/other':
|
||||
return cross_repo
|
||||
raise AssertionError(f'unexpected download: {repo_id}')
|
||||
|
||||
with mock.patch(
|
||||
'transformers.dynamic_module_utils.origin_get_class_from_dynamic_module',
|
||||
new=fake_origin,
|
||||
create=True):
|
||||
with mock.patch(
|
||||
'modelscope.snapshot_download', side_effect=fake_download):
|
||||
# Keyword form: must download and not pass duplicate positional.
|
||||
_get_class_from_dynamic_module(
|
||||
class_ref, pretrained_model_name_or_path=remote_id)
|
||||
|
||||
self.assertEqual(captured['class_reference'],
|
||||
'configuration_foo.FooConfig')
|
||||
self.assertEqual(captured['pretrained'], cross_repo)
|
||||
self.assertNotIn('pretrained_model_name_or_path', captured['kwargs'])
|
||||
|
||||
def test_import_not_pollute_dynamic_module(self):
|
||||
"""Importing from modelscope must not globally patch
|
||||
|
||||
30
tests/utils/test_owner_group_path_safety.py
Normal file
30
tests/utils/test_owner_group_path_safety.py
Normal file
@@ -0,0 +1,30 @@
|
||||
# Copyright (c) Alibaba, Inc. and its affiliates.
|
||||
import unittest
|
||||
|
||||
from modelscope.utils.automodel_utils import check_model_from_owner_group
|
||||
|
||||
|
||||
class OwnerGroupPathSafetyTest(unittest.TestCase):
|
||||
"""Safety checks for trusted-owner cache path recognition."""
|
||||
|
||||
def test_empty_name_cache_path_rejected(self):
|
||||
# modelscope_hub layout: {cache}/{owner}--{name}/snapshots/{rev}
|
||||
# Empty name ("iic--") must not be treated as a trusted owner path.
|
||||
self.assertFalse(
|
||||
check_model_from_owner_group('/cache/iic--/snapshots/v1'))
|
||||
self.assertFalse(
|
||||
check_model_from_owner_group('/cache/damo--/snapshots/v1'))
|
||||
|
||||
def test_valid_and_spoof_cache_paths(self):
|
||||
self.assertTrue(
|
||||
check_model_from_owner_group('/cache/iic--x/snapshots/v1'))
|
||||
self.assertFalse(
|
||||
check_model_from_owner_group('/cache/--iic/snapshots/v1'))
|
||||
self.assertFalse(
|
||||
check_model_from_owner_group(
|
||||
'/cache/iic--hacked--evil/snapshots/v1'))
|
||||
self.assertTrue(check_model_from_owner_group('/cache/iic/some_model'))
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
unittest.main()
|
||||
@@ -127,5 +127,40 @@ class PluginTest(unittest.TestCase):
|
||||
self.assertEqual(len(result.items()), len(OFFICIAL_PLUGINS))
|
||||
|
||||
|
||||
class ModuleNameFromModelDirTest(unittest.TestCase):
|
||||
"""Regression: snapshot revision paths must not become dotted module names."""
|
||||
|
||||
def test_snapshot_revision_with_dots(self):
|
||||
from modelscope.utils.plugins import _module_name_from_model_dir
|
||||
|
||||
tmp = tempfile.mkdtemp()
|
||||
self.addCleanup(shutil.rmtree, tmp, ignore_errors=True)
|
||||
model_dir = os.path.join(tmp, 'models',
|
||||
'iic--cv_unet_skin_retouching_torch',
|
||||
'snapshots', 'v1.0.4')
|
||||
os.makedirs(model_dir)
|
||||
|
||||
name = _module_name_from_model_dir(model_dir)
|
||||
self.assertNotIn('.', name)
|
||||
self.assertTrue(
|
||||
name.startswith('iic__cv_unet_skin_retouching_torch__v1_0_4'))
|
||||
# Old Path(model_dir).stem produced 'v1.0', which importlib treats as
|
||||
# package 'v1' → ModuleNotFoundError: No module named 'v1'.
|
||||
self.assertNotEqual(name, 'v1.0')
|
||||
self.assertFalse(name.startswith('v1'))
|
||||
|
||||
def test_flat_cache_layout_unchanged(self):
|
||||
from modelscope.utils.plugins import _module_name_from_model_dir
|
||||
|
||||
tmp = tempfile.mkdtemp()
|
||||
self.addCleanup(shutil.rmtree, tmp, ignore_errors=True)
|
||||
model_dir = os.path.join(tmp, 'iic', 'cv_unet_skin_retouching_torch')
|
||||
os.makedirs(model_dir)
|
||||
|
||||
self.assertEqual(
|
||||
_module_name_from_model_dir(model_dir),
|
||||
'cv_unet_skin_retouching_torch')
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
unittest.main()
|
||||
|
||||
Reference in New Issue
Block a user