Add run tool plugin #15
Conversation
mandar242
requested review from
GomathiselviS,
abikouo,
alinabuzachis,
beeankha and
gravesm
|
Build failed. ✔️ build-ansible-collection SUCCESS in 5m 47s |
| @@ -0,0 +1,2 @@ | |||
| # required for aws mcp server | |||
| uvx | |||
There was a problem hiding this comment.
| uvx | |
| uv |
(uvx is a command provided by the uv package)
|
Build failed. ✔️ build-ansible-collection SUCCESS in 5m 45s |
|
recheck |
|
Build failed. ❌ build-ansible-collection FAILURE in 5m 24s |
|
recheck |
|
Build failed. ✔️ build-ansible-collection SUCCESS in 5m 49s |
|
|
||
| return result | ||
|
|
||
| def _validate_connection(self): |
There was a problem hiding this comment.
Wouldn't make sense to move this into a shared utility, since this is something needed for all the action plugins?
| result["changed"] = False | ||
| result["content"] = content | ||
|
|
||
| if is_error or "isError" in response: |
There was a problem hiding this comment.
I do think you can only keep if is_error: because it already checks if isError is within response.
| if "structured_content" in response: | ||
| result["structured_content"] = response["structured_content"] | ||
|
|
||
| if is_error: |
There was a problem hiding this comment.
Doesn't make sense to move the content inside this check into line 82 https://github.com/ansible-collections/ansible.mcp/pull/15/files#diff-31f027eeccd858f79f892e99407fcd14a25b488202356001ae82cb8e50908523R82? You are already checking if is_error: at that line.
|
|
||
| # Validate connection | ||
| if error := self._validate_connection(): | ||
| result["failed"] = True |
There was a problem hiding this comment.
You could also define a dataclass for the returned result.
| f"Parameter 'args' must be a dictionary, got {type(tool_args).__name__}", | ||
| ) | ||
|
|
||
| return tool_name, tool_args, None |
There was a problem hiding this comment.
I do think for type hinting consistency, this should be probably an empty string. But I also do think, using a dedicated datatype will help.
| conn = Connection(self._connection.socket_path) | ||
| return conn.call_tool(tool_name, **tool_args) | ||
|
|
||
| def _populate_result(self, result, response, tool_name): |
There was a problem hiding this comment.
Can you also please ensure type hinting is used and same for docstrings?
3 participants
SUMMARY
https://issues.redhat.com/browse/ACA-4359
This PR adds run_tool plugin
ISSUE TYPE
COMPONENT NAME
ADDITIONAL INFORMATION