-
Notifications
You must be signed in to change notification settings - Fork 347
Extend ExecuteResourceQueryAsync to return a model type with more information #1526
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Extend ExecuteResourceQueryAsync to return a model type with more information #1526
Conversation
There was a problem hiding this 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 extends the BaseAzureResourceService.ExecuteResourceQueryAsync method to return a ResourceQueryResults<T> wrapper type that includes both the query results and a TruncatedResults boolean flag. This flag indicates whether the Azure Resource Graph query limit prevented all matching resources from being returned.
Changes:
- Introduces a new
ResourceQueryResults<T>record type withResultsandTruncatedResultsproperties - Updates
ExecuteResourceQueryAsyncinBaseAzureResourceServiceto return the new wrapper type - Updates service interfaces and implementations across 7 tool packages (ACR, AppConfig, Authorization, Grafana, Kusto, SQL, Storage)
- Updates command result records to include the
TruncatedResultsfield - Updates all unit tests to work with the new return type
- Improves test patterns by replacing
Task.FromExceptionwithThrowsAsyncfor NSubstitute mocks
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| core/Azure.Mcp.Core/src/Services/Azure/BaseAzureResourceService.cs | Adds ResourceQueryResults<T> record and updates ExecuteResourceQueryAsync return type to include truncation information |
| tools/Azure.Mcp.Tools.Storage/src/Services/IStorageService.cs | Updates GetAccountDetails signature to return ResourceQueryResults<StorageAccountInfo> |
| tools/Azure.Mcp.Tools.Storage/src/Services/StorageService.cs | Implements new return type for GetAccountDetails, returns wrapper with truncation flag |
| tools/Azure.Mcp.Tools.Sql/src/Services/ISqlService.cs | Updates ListDatabasesAsync and GetElasticPoolsAsync signatures to return ResourceQueryResults |
| tools/Azure.Mcp.Tools.Sql/src/Services/SqlService.cs | Implements new return types for database and elastic pool listing methods |
| tools/Azure.Mcp.Tools.Sql/src/Commands/Database/DatabaseListCommand.cs | Updates result record to include TruncatedResults and extracts both properties from service response |
| tools/Azure.Mcp.Tools.Sql/src/Commands/ElasticPool/ElasticPoolListCommand.cs | Updates result record to include TruncatedResults and extracts both properties from service response |
| tools/Azure.Mcp.Tools.Kusto/src/Services/IKustoService.cs | Updates ListClustersAsync signature to return ResourceQueryResults<string> |
| tools/Azure.Mcp.Tools.Kusto/src/Services/KustoService.cs | Implements new return type for cluster listing |
| tools/Azure.Mcp.Tools.Kusto/src/Commands/ClusterListCommand.cs | Updates result record and extracts truncation information |
| tools/Azure.Mcp.Tools.Grafana/src/Services/IGrafanaService.cs | Updates ListWorkspacesAsync signature to return ResourceQueryResults<GrafanaWorkspace> |
| tools/Azure.Mcp.Tools.Grafana/src/Services/GrafanaService.cs | Implements new return type for workspace listing |
| tools/Azure.Mcp.Tools.Grafana/src/Commands/Workspace/WorkspaceListCommand.cs | Updates result record and extracts truncation information |
| tools/Azure.Mcp.Tools.Authorization/src/Services/IAuthorizationService.cs | Updates ListRoleAssignmentsAsync signature to return ResourceQueryResults<RoleAssignment> |
| tools/Azure.Mcp.Tools.Authorization/src/Services/AuthorizationService.cs | Implements new return type for role assignment listing |
| tools/Azure.Mcp.Tools.Authorization/src/Commands/RoleAssignmentListCommand.cs | Updates result record and extracts truncation information |
| tools/Azure.Mcp.Tools.AppConfig/src/Services/IAppConfigService.cs | Updates GetAppConfigAccounts signature to return ResourceQueryResults<AppConfigurationAccount> |
| tools/Azure.Mcp.Tools.AppConfig/src/Services/AppConfigService.cs | Implements new return type for account listing |
| tools/Azure.Mcp.Tools.AppConfig/src/Commands/Account/AccountListCommand.cs | Updates result record and extracts truncation information |
| tools/Azure.Mcp.Tools.Acr/src/Services/IAcrService.cs | Updates ListRegistries signature to return ResourceQueryResults<AcrRegistryInfo> |
| tools/Azure.Mcp.Tools.Acr/src/Services/AcrService.cs | Implements new return type for registry listing |
| tools/Azure.Mcp.Tools.Acr/src/Commands/Registry/RegistryListCommand.cs | Updates result record and extracts truncation information |
| Various test files | Updates mocks and expectations to use ResourceQueryResults wrapper, improves test patterns with ThrowsAsync |
| servers/Azure.Mcp.Server/docs/new-command.md | Updates documentation example to show new return type |
| AGENTS.md | Updates service implementation example to show new return type |
| core/Azure.Mcp.Core/tests/.../SubscriptionCommandTests.cs | Updates tests to work with new return type |
| } | ||
| } | ||
|
|
||
| public sealed record ResourceQueryResults<T>(List<T> Results, bool TruncatedResults); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest renaming this field to make its meaning more obvious, like areResultsTruncated. TruncatedResults can be interpreted as the results themselves except that they are truncated.
What does this PR do?
Extends
BaseAzureResouceService.ExecuteResourceQueryAsyncto return a model type that can be extended with additional information in the future.In this PR,
TruncatedResultsis added to the results returned byExecuteResourceQueryAsyncto indicate if the query limit prevented results from being returned, or in other words indicating if results were truncated and additional queries are needed to get all the results. At this time this is nothing more than information for the LLM and user as acting on truncation won't be available until paging is implemented.GitHub issue number?
[Link to the GitHub issue this PR addresses]Pre-merge Checklist
servers/Azure.Mcp.Server/CHANGELOG.mdand/orservers/Fabric.Mcp.Server/CHANGELOG.mdfor product changes (features, bug fixes, UI/UX, updated dependencies)servers/Azure.Mcp.Server/README.mdand/orservers/Fabric.Mcp.Server/README.mddocumentationeng/scripts/Process-PackageReadMe.ps1. See Package README/servers/Azure.Mcp.Server/docs/azmcp-commands.mdand/or/docs/fabric-commands.md.\eng\scripts\Update-AzCommandsMetadata.ps1to update tool metadata in azmcp-commands.md (required for CI)ToolDescriptionEvaluatorand obtained a score of0.4or more and a top 3 ranking for all related test promptsconsolidated-tools.json/servers/Azure.Mcp.Server/docs/e2eTestPrompts.mdcrypto mining, spam, data exfiltration, etc.)/azp run mcp - pullrequest - liveto run Live Test Pipeline