-
Notifications
You must be signed in to change notification settings - Fork 83
code_executors: defer docker and magic imports to avoid hang on Windows #231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
bb27456
33aedbf
7f60d93
fffa3a5
7f2e29a
f2ff424
2314680
2e13abb
1e6e190
0949e38
ba298ca
d72f875
f493e18
8e676d2
85b8def
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,10 +5,13 @@ | |
| # tRPC-Agent-Python is licensed under Apache-2.0. | ||
|
|
||
| import os | ||
| import sys | ||
| import tempfile | ||
| from pathlib import Path | ||
| from unittest.mock import patch | ||
|
|
||
| import pytest | ||
|
|
||
| from trpc_agent_sdk.code_executors.utils import collect_files_with_glob | ||
| from trpc_agent_sdk.code_executors.utils import copy_dir | ||
| from trpc_agent_sdk.code_executors.utils import copy_path | ||
|
|
@@ -326,18 +329,28 @@ def test_detect_content_type_text_utf8(self): | |
|
|
||
| assert "text" in mime_type.lower() or mime_type == "application/octet-stream" | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. test_detect_content_type_with_magic 因局部 import 导致 patch 失效
|
||
| @patch('trpc_agent_sdk.code_executors.utils._files.HAS_MAGIC', True) | ||
| @patch('trpc_agent_sdk.code_executors.utils._files.magic', create=True) | ||
| def test_detect_content_type_with_magic(self, mock_magic): | ||
| @pytest.mark.skipif(sys.platform == 'win32', reason='python-magic crashes on Windows without libmagic DLL') | ||
| def test_detect_content_type_with_magic(self): | ||
| """Test detecting content type using magic library.""" | ||
| from unittest.mock import MagicMock | ||
|
|
||
| filename = Path("test.unknown") | ||
| data = b"some data" | ||
| mock_magic = MagicMock() | ||
| mock_magic.from_buffer.return_value = "application/custom" | ||
|
|
||
| mime_type = detect_content_type(filename, data) | ||
|
|
||
| assert mime_type == "application/custom" | ||
| mock_magic.from_buffer.assert_called_once_with(data, mime=True) | ||
| # Inject mock into the module's cached magic slot | ||
| import trpc_agent_sdk.code_executors.utils._files as _f | ||
| orig = (_f._magic_module, _f._magic_checked, _f.HAS_MAGIC) | ||
| _f._magic_module = mock_magic | ||
| _f._magic_checked = True | ||
| _f.HAS_MAGIC = True | ||
| try: | ||
| mime_type = detect_content_type(filename, data) | ||
| assert mime_type == "application/custom" | ||
| mock_magic.from_buffer.assert_called_once_with(data, mime=True) | ||
| finally: | ||
| _f._magic_module, _f._magic_checked, _f.HAS_MAGIC = orig | ||
|
|
||
|
|
||
| class TestGetRelPath: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,14 +14,29 @@ | |
| import mimetypes | ||
| import os | ||
| import shutil | ||
| import sys | ||
| from pathlib import Path | ||
|
|
||
| from trpc_agent_sdk.log import logger | ||
| from typing import Optional | ||
|
|
||
| try: | ||
| import magic | ||
| HAS_MAGIC = True | ||
| except ImportError: | ||
| HAS_MAGIC = False | ||
| # NOTE: python-magic is NOT imported at module top level. | ||
| # On Windows (and other platforms without libmagic installed), the `magic` | ||
| # package searches for the native libmagic DLL at import time, which can hang | ||
| # or crash the interpreter — making the entire trpc_agent_sdk package unusable. | ||
| # Instead, we lazily import it inside detect_content_type() on first use. | ||
| # See: https://github.com/trpc-group/trpc-agent-python/issues/230 | ||
| import threading as _threading | ||
|
|
||
| _magic_module = None # cached after first successful import on non-win32 | ||
| _magic_checked = False | ||
| _magic_lock = _threading.Lock() | ||
|
|
||
| # Backward-compatible public alias. Remains False until first | ||
| # detect_content_type() call triggers lazy import on non-win32. | ||
| # Note: on win32 this is always False (python-magic requires libmagic DLL | ||
| # which causes access violations at import time). | ||
| HAS_MAGIC = False | ||
|
|
||
|
|
||
| def path_join(base: str, path: str) -> str: | ||
|
|
@@ -228,9 +243,27 @@ def detect_content_type(filename: Path, data: bytes) -> str: | |
| if mime_type: | ||
| return mime_type | ||
|
|
||
| # filename guess failed, use magic to guess | ||
| if HAS_MAGIC: | ||
| return magic.from_buffer(data, mime=True) | ||
| # filename guess failed, try python-magic (lazily imported). | ||
| # On Windows, python-magic requires a native libmagic DLL that, when | ||
| # missing, causes an access violation at import time (not catchable by | ||
| # try/except). We skip it entirely on win32 and fall through to the | ||
| # simple content-based detection below. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. HAS_MAGIC 被硬编码为 False,magic 检测路径成为死代码
|
||
| global _magic_module, _magic_checked, HAS_MAGIC | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. magic 懒加载失败缓存仅捕获 ImportError,无法覆盖其它导入失败
|
||
| if sys.platform != 'win32' and not _magic_checked: | ||
| with _magic_lock: | ||
| if not _magic_checked: | ||
| try: | ||
| import magic as _m | ||
| _magic_module = _m | ||
| HAS_MAGIC = True | ||
| except Exception: | ||
| logger.debug("python-magic import failed; falling back to byte-signature detection", exc_info=True) | ||
| _magic_checked = True # cache result (success or failure) | ||
| if _magic_module is not None: | ||
| try: | ||
| return _magic_module.from_buffer(data, mime=True) | ||
| except Exception: | ||
| logger.debug("magic.from_buffer failed; falling back to byte-signature detection", exc_info=True) | ||
|
|
||
| # magic guess failed, use simple content-based detection | ||
| if data.startswith(b'\x89PNG\r\n\x1a\n'): | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
模块级副作用调用 _import_docker 污染全局状态
import 期执行 _import_docker() 会把真实 docker 符号写入 _container_cli 全局并置 _docker_imported=True,使测试行为依赖运行环境而非自洽的 mock,且 _docker_imported 置位后不可恢复。建议改在 fixture 中用 monkeypatch 设置相关符号或显式管理 _docker_imported 状态,避免 import 期执行有状态副作用。