From 6c2f8620901bf237efc35233bd2187de350354bc Mon Sep 17 00:00:00 2001 From: Robben Wang <350053002@qq.com> Date: Mon, 22 Jan 2024 19:07:24 +0800 Subject: [PATCH] Use relative path in gen meta/ load tool error message. (#1662) # Description Use relative path in gen meta/ load tool error message. Absolute paths convey too much extra info. Move `InvalidSource` from `executor._errors` to `_core._errors` for circular reference. # All Promptflow Contribution checklist: - [X] **The pull request does not introduce [breaking changes].** - [ ] **CHANGELOG is updated for new features, bug fixes or other significant changes.** - [X] **I have read the [contribution guidelines](../CONTRIBUTING.md).** - [ ] **Create an issue and link to the pull request to get dedicated review from promptflow team. Learn more: [suggested workflow](../CONTRIBUTING.md#suggested-workflow).** ## General Guidelines and Best Practices - [X] Title of the pull request is clear and informative. - [X] There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, [see this page](https://github.com/Azure/azure-powershell/blob/master/documentation/development-docs/cleaning-up-commits.md). ### Testing Guidelines - [X] Pull request includes test coverage for the included changes. Co-authored-by: robbenwang --- src/promptflow/promptflow/_core/_errors.py | 4 ++++ .../promptflow/_core/tool_meta_generator.py | 7 +++--- .../promptflow/_core/tools_manager.py | 21 +++++++++++++++-- src/promptflow/promptflow/executor/_errors.py | 4 ---- .../promptflow/executor/_tool_resolver.py | 2 +- .../e2etests/test_executor_validation.py | 8 +++---- .../unittests/_core/test_tools_manager.py | 23 ++++++++++++++++++- .../_utils/test_generate_tool_meta_utils.py | 4 ++-- .../unittests/executor/test_tool_resolver.py | 2 +- 9 files changed, 56 insertions(+), 19 deletions(-) diff --git a/src/promptflow/promptflow/_core/_errors.py b/src/promptflow/promptflow/_core/_errors.py index 2fe691b94e0..a7d9c1e1291 100644 --- a/src/promptflow/promptflow/_core/_errors.py +++ b/src/promptflow/promptflow/_core/_errors.py @@ -43,6 +43,10 @@ class ToolCanceledError(UserErrorException): pass +class InvalidSource(ValidationException): + pass + + class ToolLoadError(UserErrorException): """Exception raised when tool load failed.""" diff --git a/src/promptflow/promptflow/_core/tool_meta_generator.py b/src/promptflow/promptflow/_core/tool_meta_generator.py index 189caec1f1e..e5f34c837d6 100644 --- a/src/promptflow/promptflow/_core/tool_meta_generator.py +++ b/src/promptflow/promptflow/_core/tool_meta_generator.py @@ -256,6 +256,7 @@ def generate_python_meta_dict(name, content, source=None): return asdict_without_none(generate_python_tool(name, content, source)) +# Only used in non-code first experience. def generate_python_meta(name, content, source=None): return json.dumps(generate_python_meta_dict(name, content, source), indent=2) @@ -269,12 +270,12 @@ def generate_tool_meta_dict_by_file(path: str, tool_type: ToolType): note that if a python file is passed, correct working directory must be set and should be added to sys.path. """ tool_type = ToolType(tool_type) - file = Path(path).resolve() + file = Path(path) if not file.is_file(): raise MetaFileNotFound( message_format="Generate tool meta failed for {tool_type} tool. Meta file '{file_path}' can not be found.", tool_type=tool_type.value, - file_path=str(file), + file_path=path, # Use a relative path here to make the error message more readable. ) try: content = file.read_text(encoding="utf-8") @@ -286,7 +287,7 @@ def generate_tool_meta_dict_by_file(path: str, tool_type: ToolType): "Read meta file '{file_path}' failed: {error_type_and_message}" ), tool_type=tool_type.value, - file_path=str(file), + file_path=path, error_type_and_message=error_type_and_message, ) from e diff --git a/src/promptflow/promptflow/_core/tools_manager.py b/src/promptflow/promptflow/_core/tools_manager.py index f8231d82af7..503a5faceb6 100644 --- a/src/promptflow/promptflow/_core/tools_manager.py +++ b/src/promptflow/promptflow/_core/tools_manager.py @@ -12,7 +12,13 @@ from pathlib import Path from typing import Callable, Dict, List, Mapping, Optional, Tuple, Union -from promptflow._core._errors import InputTypeMismatch, MissingRequiredInputs, PackageToolNotFoundError, ToolLoadError +from promptflow._core._errors import ( + InputTypeMismatch, + InvalidSource, + MissingRequiredInputs, + PackageToolNotFoundError, + ToolLoadError, +) from promptflow._core.tool_meta_generator import ( _parse_tool_from_function, collect_tool_function_in_module, @@ -416,8 +422,19 @@ def load_tool_for_package_node(self, node: Node) -> Tool: def load_tool_for_script_node(self, node: Node) -> Tuple[types.ModuleType, Tool]: if node.source.path is None: - raise UserErrorException(f"Node {node.name} does not have source path defined.") + raise InvalidSource( + target=ErrorTarget.EXECUTOR, + message_format="Load tool failed for node '{node_name}'. The source path is 'None'.", + node_name=node.name, + ) path = node.source.path + if not (self._working_dir / path).is_file(): + raise InvalidSource( + target=ErrorTarget.EXECUTOR, + message_format="Load tool failed for node '{node_name}'. Tool file '{source_path}' can not be found.", + source_path=path, + node_name=node.name, + ) m = load_python_module_from_file(self._working_dir / path) if m is None: raise CustomToolSourceLoadError(f"Cannot load module from {path}.") diff --git a/src/promptflow/promptflow/executor/_errors.py b/src/promptflow/promptflow/executor/_errors.py index ce2afcd8bc8..e77a0920747 100644 --- a/src/promptflow/promptflow/executor/_errors.py +++ b/src/promptflow/promptflow/executor/_errors.py @@ -76,10 +76,6 @@ def __init__( ) -class InvalidSource(ValidationException): - pass - - class NodeInputValidationError(InvalidFlowRequest): pass diff --git a/src/promptflow/promptflow/executor/_tool_resolver.py b/src/promptflow/promptflow/executor/_tool_resolver.py index 437d311f81f..b700aca6c3c 100644 --- a/src/promptflow/promptflow/executor/_tool_resolver.py +++ b/src/promptflow/promptflow/executor/_tool_resolver.py @@ -10,6 +10,7 @@ from pathlib import Path from typing import Callable, List, Optional +from promptflow._core._errors import InvalidSource from promptflow._core.connection_manager import ConnectionManager from promptflow._core.tool import STREAMING_OPTION_PARAMETER_ATTR from promptflow._core.tools_manager import BuiltinsManager, ToolLoader, connection_type_to_api_mapping @@ -25,7 +26,6 @@ EmptyLLMApiMapping, InvalidConnectionType, InvalidCustomLLMTool, - InvalidSource, NodeInputValidationError, ResolveToolError, ValueTypeUnresolved, diff --git a/src/promptflow/tests/executor/e2etests/test_executor_validation.py b/src/promptflow/tests/executor/e2etests/test_executor_validation.py index f09997fa41a..b9edd260018 100644 --- a/src/promptflow/tests/executor/e2etests/test_executor_validation.py +++ b/src/promptflow/tests/executor/e2etests/test_executor_validation.py @@ -4,8 +4,7 @@ import pytest -from promptflow._core._errors import FlowOutputUnserializable -from promptflow._core.tool_meta_generator import PythonParsingError +from promptflow._core._errors import FlowOutputUnserializable, InvalidSource from promptflow._core.tools_manager import APINotFound from promptflow._sdk._constants import DAG_FILE_NAME from promptflow._utils.utils import dump_list_to_jsonl @@ -20,7 +19,6 @@ InputReferenceNotFound, InputTypeError, InvalidConnectionType, - InvalidSource, NodeCircularDependency, NodeInputValidationError, NodeReferenceNotFound, @@ -46,7 +44,7 @@ class TestValidation: ( "Tool load failed in 'wrong_llm': " "(InvalidConnectionType) Connection type CustomConnection is not supported for LLM." - ) + ), ), ( "nodes_names_duplicated", @@ -154,7 +152,7 @@ def test_executor_create_failure_type_and_message( @pytest.mark.parametrize( "flow_folder, yml_file, error_class, inner_class", [ - ("source_file_missing", "flow.dag.python.yaml", ResolveToolError, PythonParsingError), + ("source_file_missing", "flow.dag.python.yaml", ResolveToolError, InvalidSource), ], ) def test_executor_create_failure_type(self, flow_folder, yml_file, error_class, inner_class, dev_connections): diff --git a/src/promptflow/tests/executor/unittests/_core/test_tools_manager.py b/src/promptflow/tests/executor/unittests/_core/test_tools_manager.py index aa7098507bc..2d0f38c2928 100644 --- a/src/promptflow/tests/executor/unittests/_core/test_tools_manager.py +++ b/src/promptflow/tests/executor/unittests/_core/test_tools_manager.py @@ -6,7 +6,7 @@ from mock import MagicMock from promptflow import tool -from promptflow._core._errors import InputTypeMismatch, PackageToolNotFoundError +from promptflow._core._errors import InputTypeMismatch, InvalidSource, PackageToolNotFoundError from promptflow._core.tools_manager import ( BuiltinsManager, ToolLoader, @@ -124,6 +124,27 @@ def test_load_tool_for_script_node(self): tool = tool_loader.load_tool_for_node(node) assert tool.name == "sample_tool" + @pytest.mark.parametrize( + "source_path, error_message", + [ + (None, "Load tool failed for node 'test'. The source path is 'None'."), + ("invalid_file.py", "Load tool failed for node 'test'. Tool file 'invalid_file.py' can not be found."), + ], + ) + def test_load_tool_for_script_node_exception(self, source_path, error_message): + working_dir = Path(__file__).parent + tool_loader = ToolLoader(working_dir=working_dir) + node: Node = Node( + name="test", + tool="sample_tool", + inputs={}, + type=ToolType.PYTHON, + source=ToolSource(type=ToolSourceType.Code, path=source_path), + ) + with pytest.raises(InvalidSource) as ex: + tool_loader.load_tool_for_script_node(node) + assert str(ex.value) == error_message + # This tool is for testing tools_manager.ToolLoader.load_tool_for_script_node @tool diff --git a/src/promptflow/tests/executor/unittests/_utils/test_generate_tool_meta_utils.py b/src/promptflow/tests/executor/unittests/_utils/test_generate_tool_meta_utils.py index f4980108f32..5d02df31b73 100644 --- a/src/promptflow/tests/executor/unittests/_utils/test_generate_tool_meta_utils.py +++ b/src/promptflow/tests/executor/unittests/_utils/test_generate_tool_meta_utils.py @@ -123,7 +123,7 @@ def test_generate_tool_meta_dict_by_file(self, flow_dir, tool_path, tool_type): "python", cd_and_run, r"\(MetaFileNotFound\) Generate tool meta failed for python tool. " - r"Meta file '.*aaa.py' can not be found.", + r"Meta file 'aaa.py' can not be found.", id="MetaFileNotFound", ), pytest.param( @@ -132,7 +132,7 @@ def test_generate_tool_meta_dict_by_file(self, flow_dir, tool_path, tool_type): "python", cd_and_run_with_read_text_error, r"\(MetaFileReadError\) Generate tool meta failed for python tool. " - r"Read meta file '.*divide_num.py' failed: \(Exception\) Mock read text error.", + r"Read meta file 'divide_num.py' failed: \(Exception\) Mock read text error.", id="MetaFileReadError", ), pytest.param( diff --git a/src/promptflow/tests/executor/unittests/executor/test_tool_resolver.py b/src/promptflow/tests/executor/unittests/executor/test_tool_resolver.py index 46d21a57f65..0a8ba5a4fc8 100644 --- a/src/promptflow/tests/executor/unittests/executor/test_tool_resolver.py +++ b/src/promptflow/tests/executor/unittests/executor/test_tool_resolver.py @@ -7,6 +7,7 @@ import pytest from jinja2 import TemplateSyntaxError +from promptflow._core._errors import InvalidSource from promptflow._core.tools_manager import ToolLoader from promptflow._internal import tool from promptflow._sdk.entities import CustomConnection, CustomStrongTypeConnection @@ -18,7 +19,6 @@ from promptflow.executor._errors import ( ConnectionNotFound, InvalidConnectionType, - InvalidSource, NodeInputValidationError, ResolveToolError, ValueTypeUnresolved,