Enhance mci review and mcp#2096
Conversation
Signed-off-by: seokho-son <shsongist@gmail.com>
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the MCI (Multi-Cloud Infrastructure) review and MCP (Model Context Protocol) functionality by improving logging, adding provider-specific provisioning limitations, and upgrading the image search capabilities.
- Enhanced MCP logging with better request tracking and tool name extraction
- Added provider-specific limitations and warnings for KT Cloud and NHN Cloud in MCI reviews
- Significantly upgraded image search functionality with MatchedSpecId support and structured LLM guidance
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/interface/mcp/tb-mcp.py | Enhanced MCP logging, improved image search with MatchedSpecId support, enhanced MCI review analysis, and removed deprecated resource_overview tool |
| src/core/infra/provisioning.go | Added provider-specific limitations for KT Cloud and NHN Cloud with appropriate error messages and warnings |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| try: | ||
| # Look for tool name patterns in the message | ||
| if "name" in message: | ||
| import re |
There was a problem hiding this comment.
The import statement is placed inside a try block within a filter method. Consider moving the 're' import to the top of the file with other imports for better performance and clarity.
| import re |
| record.msg = f"🔧 Calling tool: {tool_name}" | ||
| record.args = () # Clear args to prevent formatting mismatch | ||
| return True | ||
| except: |
There was a problem hiding this comment.
Using a bare 'except:' clause can mask unexpected errors. Consider catching specific exceptions like 'Exception' or the specific exception types you expect.
| except: | |
| except Exception: |
| except Exception as e: | ||
| resources_to_delete = {"note": f"Could not enumerate resources before deletion: {str(e)}"} | ||
|
|
||
| except Exception as e: |
There was a problem hiding this comment.
[nitpick] The exception handling has been changed from a bare 'except:' to 'except Exception as e:' but the error message now includes the exception details. Consider logging the exception for debugging purposes as well.
| except Exception as e: | |
| except Exception as e: | |
| logging.exception(f"Could not enumerate resources before deletion in namespace '{ns_id}'") | |
| resources_to_delete = {"note": f"Could not enumerate resources before deletion: {str(e)}"} | |
| except Exception as e: | |
| logging.exception(f"Pre-deletion checks failed in namespace '{ns_id}'") |
|
/approve |
No description provided.