Skip to content

Commit

Permalink
Use relative path in gen meta/ load tool error message. (#1662)
Browse files Browse the repository at this point in the history
# 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 <[email protected]>
  • Loading branch information
huaiyan and robbenwang authored Jan 22, 2024
1 parent ed29b8b commit 6c2f862
Show file tree
Hide file tree
Showing 9 changed files with 56 additions and 19 deletions.
4 changes: 4 additions & 0 deletions src/promptflow/promptflow/_core/_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ class ToolCanceledError(UserErrorException):
pass


class InvalidSource(ValidationException):
pass


class ToolLoadError(UserErrorException):
"""Exception raised when tool load failed."""

Expand Down
7 changes: 4 additions & 3 deletions src/promptflow/promptflow/_core/tool_meta_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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")
Expand All @@ -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

Expand Down
21 changes: 19 additions & 2 deletions src/promptflow/promptflow/_core/tools_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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}.")
Expand Down
4 changes: 0 additions & 4 deletions src/promptflow/promptflow/executor/_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,6 @@ def __init__(
)


class InvalidSource(ValidationException):
pass


class NodeInputValidationError(InvalidFlowRequest):
pass

Expand Down
2 changes: 1 addition & 1 deletion src/promptflow/promptflow/executor/_tool_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -25,7 +26,6 @@
EmptyLLMApiMapping,
InvalidConnectionType,
InvalidCustomLLMTool,
InvalidSource,
NodeInputValidationError,
ResolveToolError,
ValueTypeUnresolved,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -20,7 +19,6 @@
InputReferenceNotFound,
InputTypeError,
InvalidConnectionType,
InvalidSource,
NodeCircularDependency,
NodeInputValidationError,
NodeReferenceNotFound,
Expand All @@ -46,7 +44,7 @@ class TestValidation:
(
"Tool load failed in 'wrong_llm': "
"(InvalidConnectionType) Connection type CustomConnection is not supported for LLM."
)
),
),
(
"nodes_names_duplicated",
Expand Down Expand Up @@ -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):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -18,7 +19,6 @@
from promptflow.executor._errors import (
ConnectionNotFound,
InvalidConnectionType,
InvalidSource,
NodeInputValidationError,
ResolveToolError,
ValueTypeUnresolved,
Expand Down

0 comments on commit 6c2f862

Please sign in to comment.