mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-05-01 07:13:00 +00:00
fix: Use sha256 instead of md5 for module name hashing (lint S324)
- Add missing mocks to metadata extraction failure test
This commit is contained in:
@@ -1,4 +1,5 @@
|
|||||||
from collections.abc import Mapping
|
from collections.abc import Mapping
|
||||||
|
from contextlib import contextmanager
|
||||||
from functools import lru_cache, reduce
|
from functools import lru_cache, reduce
|
||||||
import hashlib
|
import hashlib
|
||||||
import importlib.util
|
import importlib.util
|
||||||
@@ -547,43 +548,58 @@ def build_env_with_tool_repository_credentials(
|
|||||||
return env
|
return env
|
||||||
|
|
||||||
|
|
||||||
|
@contextmanager
|
||||||
|
def _load_module_from_file(init_file: Path, module_name: str | None = None):
|
||||||
|
"""
|
||||||
|
Context manager for loading a module from file with automatic cleanup.
|
||||||
|
|
||||||
|
Yields the loaded module or None if loading fails.
|
||||||
|
"""
|
||||||
|
if module_name is None:
|
||||||
|
module_name = f"temp_module_{hashlib.sha256(str(init_file).encode()).hexdigest()[:8]}"
|
||||||
|
|
||||||
|
spec = importlib.util.spec_from_file_location(module_name, init_file)
|
||||||
|
if not spec or not spec.loader:
|
||||||
|
yield None
|
||||||
|
return
|
||||||
|
|
||||||
|
module = importlib.util.module_from_spec(spec)
|
||||||
|
sys.modules[module_name] = module
|
||||||
|
|
||||||
|
try:
|
||||||
|
spec.loader.exec_module(module)
|
||||||
|
yield module
|
||||||
|
finally:
|
||||||
|
sys.modules.pop(module_name, None)
|
||||||
|
|
||||||
|
|
||||||
def _load_tools_from_init(init_file: Path) -> list[dict[str, Any]]:
|
def _load_tools_from_init(init_file: Path) -> list[dict[str, Any]]:
|
||||||
"""
|
"""
|
||||||
Load and validate tools from a given __init__.py file.
|
Load and validate tools from a given __init__.py file.
|
||||||
"""
|
"""
|
||||||
spec = importlib.util.spec_from_file_location("temp_module", init_file)
|
|
||||||
|
|
||||||
if not spec or not spec.loader:
|
|
||||||
return []
|
|
||||||
|
|
||||||
module = importlib.util.module_from_spec(spec)
|
|
||||||
sys.modules["temp_module"] = module
|
|
||||||
|
|
||||||
try:
|
try:
|
||||||
spec.loader.exec_module(module)
|
with _load_module_from_file(init_file) as module:
|
||||||
|
if module is None:
|
||||||
|
return []
|
||||||
|
|
||||||
if not hasattr(module, "__all__"):
|
if not hasattr(module, "__all__"):
|
||||||
console.print(
|
console.print(
|
||||||
f"Warning: No __all__ defined in {init_file}",
|
f"Warning: No __all__ defined in {init_file}",
|
||||||
style="bold yellow",
|
style="bold yellow",
|
||||||
)
|
)
|
||||||
raise SystemExit(1)
|
raise SystemExit(1)
|
||||||
|
|
||||||
return [
|
|
||||||
{
|
|
||||||
"name": name,
|
|
||||||
}
|
|
||||||
for name in module.__all__
|
|
||||||
if hasattr(module, name) and is_valid_tool(getattr(module, name))
|
|
||||||
]
|
|
||||||
|
|
||||||
|
return [
|
||||||
|
{"name": name}
|
||||||
|
for name in module.__all__
|
||||||
|
if hasattr(module, name) and is_valid_tool(getattr(module, name))
|
||||||
|
]
|
||||||
|
except SystemExit:
|
||||||
|
raise
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
console.print(f"[red]Warning: Could not load {init_file}: {e!s}[/red]")
|
console.print(f"[red]Warning: Could not load {init_file}: {e!s}[/red]")
|
||||||
raise SystemExit(1) from e
|
raise SystemExit(1) from e
|
||||||
|
|
||||||
finally:
|
|
||||||
sys.modules.pop("temp_module", None)
|
|
||||||
|
|
||||||
|
|
||||||
def _print_no_tools_warning() -> None:
|
def _print_no_tools_warning() -> None:
|
||||||
"""
|
"""
|
||||||
@@ -642,41 +658,30 @@ def _extract_tool_metadata_from_init(init_file: Path) -> list[dict[str, Any]]:
|
|||||||
"""
|
"""
|
||||||
from crewai.tools.base_tool import BaseTool
|
from crewai.tools.base_tool import BaseTool
|
||||||
|
|
||||||
module_name = f"temp_metadata_{hashlib.sha256(str(init_file).encode()).hexdigest()[:8]}"
|
|
||||||
spec = importlib.util.spec_from_file_location(module_name, init_file)
|
|
||||||
|
|
||||||
if not spec or not spec.loader:
|
|
||||||
return []
|
|
||||||
|
|
||||||
module = importlib.util.module_from_spec(spec)
|
|
||||||
sys.modules[module_name] = module
|
|
||||||
|
|
||||||
try:
|
try:
|
||||||
spec.loader.exec_module(module)
|
with _load_module_from_file(init_file) as module:
|
||||||
|
if module is None:
|
||||||
|
return []
|
||||||
|
|
||||||
exported_names = getattr(module, "__all__", None)
|
exported_names = getattr(module, "__all__", None)
|
||||||
if not exported_names:
|
if not exported_names:
|
||||||
return []
|
return []
|
||||||
|
|
||||||
tools_metadata = []
|
tools_metadata = []
|
||||||
for name in exported_names:
|
for name in exported_names:
|
||||||
obj = getattr(module, name, None)
|
obj = getattr(module, name, None)
|
||||||
if obj is None or not (inspect.isclass(obj) and issubclass(obj, BaseTool)):
|
if obj is None or not (inspect.isclass(obj) and issubclass(obj, BaseTool)):
|
||||||
continue
|
continue
|
||||||
if tool_info := _extract_single_tool_metadata(obj):
|
if tool_info := _extract_single_tool_metadata(obj):
|
||||||
tools_metadata.append(tool_info)
|
tools_metadata.append(tool_info)
|
||||||
|
|
||||||
return tools_metadata
|
|
||||||
|
|
||||||
|
return tools_metadata
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
console.print(
|
console.print(
|
||||||
f"[yellow]Warning: Could not extract metadata from {init_file}: {e}[/yellow]"
|
f"[yellow]Warning: Could not extract metadata from {init_file}: {e}[/yellow]"
|
||||||
)
|
)
|
||||||
return []
|
return []
|
||||||
|
|
||||||
finally:
|
|
||||||
sys.modules.pop(module_name, None)
|
|
||||||
|
|
||||||
|
|
||||||
def _extract_single_tool_metadata(tool_class: type) -> dict[str, Any] | None:
|
def _extract_single_tool_metadata(tool_class: type) -> dict[str, Any] | None:
|
||||||
"""
|
"""
|
||||||
|
|||||||
@@ -387,6 +387,14 @@ def test_publish_api_error(
|
|||||||
@patch("crewai.cli.tools.main.get_project_name", return_value="sample-tool")
|
@patch("crewai.cli.tools.main.get_project_name", return_value="sample-tool")
|
||||||
@patch("crewai.cli.tools.main.get_project_version", return_value="1.0.0")
|
@patch("crewai.cli.tools.main.get_project_version", return_value="1.0.0")
|
||||||
@patch("crewai.cli.tools.main.get_project_description", return_value="A sample tool")
|
@patch("crewai.cli.tools.main.get_project_description", return_value="A sample tool")
|
||||||
|
@patch("crewai.cli.tools.main.subprocess.run")
|
||||||
|
@patch("crewai.cli.tools.main.os.listdir", return_value=["sample-tool-1.0.0.tar.gz"])
|
||||||
|
@patch(
|
||||||
|
"crewai.cli.tools.main.open",
|
||||||
|
new_callable=unittest.mock.mock_open,
|
||||||
|
read_data=b"sample tarball content",
|
||||||
|
)
|
||||||
|
@patch("crewai.cli.plus_api.PlusAPI.publish_tool")
|
||||||
@patch("crewai.cli.tools.main.git.Repository.is_synced", return_value=True)
|
@patch("crewai.cli.tools.main.git.Repository.is_synced", return_value=True)
|
||||||
@patch(
|
@patch(
|
||||||
"crewai.cli.tools.main.extract_available_exports",
|
"crewai.cli.tools.main.extract_available_exports",
|
||||||
@@ -400,6 +408,10 @@ def test_publish_metadata_extraction_failure_continues_with_warning(
|
|||||||
mock_tools_metadata,
|
mock_tools_metadata,
|
||||||
mock_available_exports,
|
mock_available_exports,
|
||||||
mock_is_synced,
|
mock_is_synced,
|
||||||
|
mock_publish,
|
||||||
|
mock_open,
|
||||||
|
mock_listdir,
|
||||||
|
mock_subprocess_run,
|
||||||
mock_get_project_description,
|
mock_get_project_description,
|
||||||
mock_get_project_version,
|
mock_get_project_version,
|
||||||
mock_get_project_name,
|
mock_get_project_name,
|
||||||
@@ -407,14 +419,26 @@ def test_publish_metadata_extraction_failure_continues_with_warning(
|
|||||||
tool_command,
|
tool_command,
|
||||||
):
|
):
|
||||||
"""Test that metadata extraction failure shows warning but continues publishing."""
|
"""Test that metadata extraction failure shows warning but continues publishing."""
|
||||||
try:
|
mock_publish_response = MagicMock()
|
||||||
tool_command.publish(is_public=True)
|
mock_publish_response.status_code = 200
|
||||||
except SystemExit:
|
mock_publish_response.json.return_value = {"handle": "sample-tool"}
|
||||||
pass # May fail later due to API mock, but should get past metadata extraction
|
mock_publish.return_value = mock_publish_response
|
||||||
|
|
||||||
|
tool_command.publish(is_public=True)
|
||||||
|
|
||||||
output = capsys.readouterr().out
|
output = capsys.readouterr().out
|
||||||
assert "Warning: Could not extract tool metadata" in output
|
assert "Warning: Could not extract tool metadata" in output
|
||||||
assert "Publishing will continue without detailed metadata" in output
|
assert "Publishing will continue without detailed metadata" in output
|
||||||
assert "No tool metadata extracted" in output
|
assert "No tool metadata extracted" in output
|
||||||
|
mock_publish.assert_called_once_with(
|
||||||
|
handle="sample-tool",
|
||||||
|
is_public=True,
|
||||||
|
version="1.0.0",
|
||||||
|
description="A sample tool",
|
||||||
|
encoded_file=unittest.mock.ANY,
|
||||||
|
available_exports=[{"name": "SampleTool"}],
|
||||||
|
tools_metadata=[],
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@patch("crewai.cli.tools.main.Settings")
|
@patch("crewai.cli.tools.main.Settings")
|
||||||
|
|||||||
Reference in New Issue
Block a user