created better throw exception logic for tools / tool_generation.py #143
Conversation
remove any sys.exit on business logic level for tools, and only raise exceptions. let cli/tools.py handle any errors and do sys.exit
|
Thanks for your contribution @yjgoh28 We should also be catching the exceptions in |
|
Got it 🫡 |
|
I was just browsing through the code today, and saw there were some unexpected sys.exits where I didn't expect to see them. |
agentstack/cli/tools.py
Outdated
| except ToolError: | ||
| print(term_color("Could not retrieve list of tools. The tools directory may be corrupted or missing.", 'red')) | ||
| sys.exit(1) | ||
| except ValidationError: |
There was a problem hiding this comment.
in only the case of a validation error, it'd be smart to log the exception because it will help them track down what's failing in the validation of their tool config.
| tools_init.add_import_for_tool(tool, agentstack_config.framework) | ||
| except ValidationError as e: | ||
| print(term_color(f"Error adding tool:\n{e}", 'red')) | ||
| raise ToolError(f"Error adding tool:\n{e}") |
There was a problem hiding this comment.
if i'm not missing something, ValidationError will never be surfaced to tools.py for the proper logging because we're catching it here and re-raising it as a ToolError
addresses in only the case of a validation error, it'd be smart to log the exception because it will help them track down what's failing in the validation of their tool config.
addresses " if i'm not missing something, ValidationError will never be surfaced to tools.py for the proper logging because we're catching it here and re-raising it as a ToolError"
|
hey @yjgoh28! we're in the middle of some sizable repo changes and plan to come back to this after a few key decisions. thank you for the effort in this PR! :) |
|
these changes are not compatible with the new design for tools - im closing this but thank you for the contribution 🫶 |
4 participants
remove any sys.exit on business logic level for tools, and only raise exceptions. let cli/tools.py handle any errors and do sys.exit
📥 Pull Request
📘 Description
addreses #141 only for tools generation
remove any sys.exit on business logic level for tools, and only raise exceptions. let cli/tools.py handle any errors and do sys.exit
🧪 Testing
N/A