Tools reorganization and error handling #40
Conversation
cbbayburt
force-pushed
the
tool-reorganization
branch
2 times, most recently
from
0c66c51 to
144a2b0
Compare
cbbayburt
force-pushed
the
tool-reorganization
branch
from
144a2b0 to
1c6b197
Compare
ycedres
left a comment
ycedres
left a comment
There was a problem hiding this comment.
LGTM. Just left small comments.
src/mcp_server_uyuni/server.py
Outdated
| system_identifier: The unique identifier of the system. It can be the system name (e.g. "buildhost") or the system ID (e.g. 1000010000). | ||
| system_identifier: The system name (e.g., "buildhost.example.com") or system ID (e.g., 1000010000). | ||
| Prefer using numerical system IDs instead of system names when possible. | ||
| offset: Number of reults to skip |
There was a problem hiding this comment.
Just a small typo in the word "results".
| @mcp.tool() | ||
| async def get_cpu_of_a_system(system_identifier: Union[str, int], ctx: Context) -> Dict[str, Any]: | ||
| async def get_system_event_history(system_identifier: Union[str, int], ctx: Context, offset: int = 0, limit: int = 10, earliest_date: str = None): | ||
| """Gets the event/action history of the specified system. |
There was a problem hiding this comment.
I tried it and works fine, the LLM just returns the first 10 events. Would it be nice to steer the LLM so it asks if the user wants the next batch with a subsequent call with the next offset?
There was a problem hiding this comment.
I would leave it to LLMs decision. It mostly depends on the history of the conversation, and what led to this tool call. In many cases, this tool would be called as part of a bigger conversation, such as "check what went wrong with this patch application on this date" or "check if my system tasks executed properly" etc. In this case, LLM should decide what slice it wants to get depending on the situation.
One thing that's missing though, is the total number of events available. This would be very useful for the LLM to know in advance. However, it's not provided by the API as of now.
|
I love it ! However, the first commit is from #33 and I have some concerns on that one, which means there might need to be a rebase. |
concerns resolved. Approving! |
cbbayburt
force-pushed
the
tool-reorganization
branch
from
c5ffa02 to
82a315d
Compare
List of changes
get_system_detailstoolget_system_detailstoolFixes https://github.com/SUSE/spacewalk/issues/27816