Skip to content
This repository was archived by the owner on Sep 4, 2025. It is now read-only.

Conversation

@ArthurMa1978
Copy link
Member

@ArthurMa1978 ArthurMa1978 commented Aug 12, 2025

What does this PR do?

This PR addresses part of the issue tracked in #756 by updating the SQL tool to eliminate its dependency on Azure.ResourceManager.Sql.
As a result, it reduces the AzureMCP agent size by approximately 6.41 MB.

GitHub issue number?

#756

Pre-merge Checklist

  • Required for All PRs
    • Read contribution guidelines
    • PR title clearly describes the change
    • Commit history is clean with descriptive messages (cleanup guide)
    • Added comprehensive tests for new/modified functionality
    • Updated CHANGELOG.md for product changes (features, bug fixes, UI/UX, updated dependencies)
    • Spelling check passes: .\eng\common\spelling\Invoke-Cspell.ps1
  • For MCP tool changes:
    • Updated README.md documentation
    • Updated command list in /docs/azmcp-commands.md
    • Updated test prompts in /e2eTests/e2eTestPrompts.md
    • For new or modified tool descriptions, ran the eng/tools/ToolDescriptionConfidenceScore tool and obtained a result >= 0.4
  • 👉 For Community (non-Azure team member) PRs:
    • Security review: Reviewed code for security vulnerabilities, malicious code, or suspicious activities before running tests (crypto mining, spam, data exfiltration, etc.)
    • Manual tests run: added comment /azp run azure - mcp to run Live Test Pipeline

Copilot AI review requested due to automatic review settings August 12, 2025 14:00
@ArthurMa1978 ArthurMa1978 requested a review from a team as a code owner August 12, 2025 14:00
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the SQL tool to use Azure Resource Graph (ARG) queries instead of direct Azure Resource Manager (ARM) operations, eliminating the dependency on Azure.ResourceManager.Sql package and reducing the agent size by ~6.41 MB.

  • Introduces BaseAzureResourceService for common Resource Graph query operations
  • Replaces ARM-based SQL operations with Resource Graph queries
  • Creates internal model classes to handle JSON deserialization from Resource Graph responses

Reviewed Changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
core/src/AzureMcp.Core/Services/Azure/BaseAzureResourceService.cs New base service class providing common Resource Graph query functionality
areas/sql/src/AzureMcp.Sql/Services/SqlService.cs Refactored to use BaseAzureResourceService and Resource Graph queries
areas/sql/src/AzureMcp.Sql/Services/Models/*.cs New internal model classes for deserializing SQL resource data from JSON
docs/new-command.md Updated documentation explaining when to use Resource Graph vs direct ARM operations
areas/sql/tests/AzureMcp.Sql.LiveTests/SqlCommandTests.cs Updated tests to use case-insensitive string comparisons

@jongio
Copy link
Member

jongio commented Aug 12, 2025

@copilot review this PR against checklist and guidance from new-command.md

@jongio jongio requested a review from Copilot August 12, 2025 16:51
Copy link
Member

@jongio jongio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added consolidated review feedback referencing new-command.md checklist.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@ArthurMa1978 ArthurMa1978 requested a review from a team as a code owner August 13, 2025 06:14
@github-project-automation github-project-automation bot moved this from Untriaged to In Progress in Azure MCP Server (OLD) Aug 13, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants