tsp-client update #41853
tsp-client update #41853
Conversation
There was a problem hiding this comment.
Pull Request Overview
Adds support for a new update command in the MCP tool and relaxes the Python version requirement.
- Broadens
requires-pythonto allow any 3.10+ interpreter - Introduces an
update_toolfunction to run a TypeSpec update command - Provides exception handling for the new update path
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| eng/tools/mcp/azure-sdk-python-mcp/pyproject.toml | Changed Python requirement from ==3.10.* to >=3.10 |
| eng/tools/mcp/azure-sdk-python-mcp/main.py | Added update_tool decorated with @mcp.tool("update") |
Comments suppressed due to low confidence (1)
eng/tools/mcp/azure-sdk-python-mcp/main.py:253
- No tests cover the new
updatecommand. Add unit tests forupdate_toolto verify successful command invocation and proper error handling.
@mcp.tool("update")
| return run_command(["update"], cwd=package_path, is_typespec=True, | ||
| typespec_args={"commit": commit_hash}) |
There was a problem hiding this comment.
The error-handling path returns a dict with a message key, but the success path (from run_command) omits this field, resulting in an inconsistent schema. Consider normalizing the response to always include the same keys (e.g., add message or rename fields consistently).
| return run_command(["update"], cwd=package_path, is_typespec=True, | |
| typespec_args={"commit": commit_hash}) | |
| result = run_command(["update"], cwd=package_path, is_typespec=True, | |
| typespec_args={"commit": commit_hash}) | |
| result["message"] = "Operation completed successfully" | |
| return result |
Copilot uses AI. Check for mistakes.
|
|
||
| @mcp.tool("update") | ||
| def update_tool(commit_hash: str, package_path:str) -> Dict[str, Any]: | ||
| """Initializes and subsequently generates a typespec client library directory from a local azure-rest-api-specs repo. |
There was a problem hiding this comment.
Nit:
| """Initializes and subsequently generates a typespec client library directory from a local azure-rest-api-specs repo. | |
| """Updates a client library from a local azure-rest-api-specs repo. |
| def update_tool(commit_hash: str, package_path:str) -> Dict[str, Any]: | ||
| """Initializes and subsequently generates a typespec client library directory from a local azure-rest-api-specs repo. | ||
|
|
||
| This command is used to update a client library to a specific commit hash in the azure-rest-api-specs repository. |
There was a problem hiding this comment.
This is kind of contradicting the previous description. Are we updating based on a local spec or a specific commit hash in the repo? It's fine (and probably ideal) if this tool can do both but we should clarify what the purpose of this tool is
| This command is used to update a client library to a specific commit hash in the azure-rest-api-specs repository. | ||
|
|
||
| Args: | ||
| commit_hash: The commit hash to update to. |
There was a problem hiding this comment.
If the local spec repo path is also supported then this should be optional
|
|
||
| Args: | ||
| commit_hash: The commit hash to update to. | ||
| package_path: The path to the directory of the tsp-location.yaml (i.e. ./azure-sdk-for-python/sdk/eventgrid/azure-eventgrid). |
There was a problem hiding this comment.
This should be optional. What if someone is asking the agent to update their client library from the same client library directory? Maybe we should check if tsp-location.yaml is in the same directory first, then if it isnt we can ask for the package path, this will make it more robust against different scenarios.
No description provided.