Skip to content

Comments

Great Cancellation: Updating New-Command.md file to include#1768

Open
msalaman wants to merge 3 commits intomainfrom
masalama/cancellationTokenFollowup
Open

Great Cancellation: Updating New-Command.md file to include#1768
msalaman wants to merge 3 commits intomainfrom
masalama/cancellationTokenFollowup

Conversation

@msalaman
Copy link
Contributor

@msalaman msalaman commented Feb 19, 2026

What does this PR do?

Improved new-command doc to include cancellationToken in appropriate scenarios. Also added cancellationToken to methods where the parameter exists but was unused.

GitHub issue number?

#1583

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 servers/Azure.Mcp.Server/CHANGELOG.md and/or servers/Fabric.Mcp.Server/CHANGELOG.md for product changes (features, bug fixes, UI/UX, updated dependencies)
  • For MCP tool changes:
    • One tool per PR: This PR adds or modifies only one MCP tool for faster review cycles
    • Updated servers/Azure.Mcp.Server/README.md and/or servers/Fabric.Mcp.Server/README.md documentation
    • Validate README.md changes using script at eng/scripts/Process-PackageReadMe.ps1. See Package README
    • Updated command list in /servers/Azure.Mcp.Server/docs/azmcp-commands.md and/or /docs/fabric-commands.md
    • Run .\eng\scripts\Update-AzCommandsMetadata.ps1 to update tool metadata in azmcp-commands.md (required for CI)
    • For new or modified tool descriptions, ran ToolDescriptionEvaluator and obtained a score of 0.4 or more and a top 3 ranking for all related test prompts
    • For tools with new names, including new tools or renamed tools, update consolidated-tools.json
    • For new tools associated with Azure services or publicly available tools/APIs/products, add URL to documentation in the PR description
  • Extra steps for Azure MCP Server tool changes:
    • Updated test prompts in /servers/Azure.Mcp.Server/docs/e2eTestPrompts.md
    • 👉 For Community (non-Microsoft 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 mcp - pullrequest - live to run Live Test Pipeline

@msalaman msalaman marked this pull request as ready for review February 19, 2026 20:16
Copilot AI review requested due to automatic review settings February 19, 2026 20:16
@msalaman msalaman requested review from a team, jongio, qianwens and xiangyan99 as code owners February 19, 2026 20:16
Copilot AI reviewed Feb 19, 2026
View reviewed changes
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 pull request completes the "Great Cancellation" initiative (issue #1583) by adding cancellationToken parameters to methods where they were present but unused, and updating the new-command.md documentation to reflect proper cancellationToken usage patterns throughout Azure SDK method calls.

Changes:

  • Added cancellationToken parameters to CreateArmClientWithApiVersionAsync and CreateOrUpdateGenericResourceAsync in BaseAzureResourceService
  • Updated callers in Storage, Foundry, and Quota tools to pass cancellationToken through the call chain
  • Enhanced documentation with comprehensive examples showing correct named parameter syntax for cancellationToken usage

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
core/Microsoft.Mcp.Core/src/Services/Azure/BaseAzureResourceService.cs Added cancellationToken parameter to CreateArmClientWithApiVersionAsync and CreateOrUpdateGenericResourceAsync methods
tools/Azure.Mcp.Tools.Storage/src/Services/StorageService.cs Updated CreateStorageAccount to pass cancellationToken to CreateArmClientWithApiVersionAsync and CreateOrUpdateGenericResourceAsync
tools/Azure.Mcp.Tools.Foundry/src/Services/FoundryService.cs Updated DeployModel to pass cancellationToken to CreateArmClientWithApiVersionAsync and CreateOrUpdateGenericResourceAsync
tools/Azure.Mcp.Tools.Quota/src/Services/Util/AzureRegionChecker.cs Added cancellationToken to GetModelsAsync and ExecuteLocationBasedCapabilitiesAsync calls
servers/Azure.Mcp.Server/docs/new-command.md Updated documentation examples to consistently show cancellationToken usage with named parameter syntax

servers/Azure.Mcp.Server/docs/new-command.md
options.Subscription!,
options.ResourceGroup,
options.RetryPolicy);
options.RetryPolicy
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

Missing comma after 'options.RetryPolicy' parameter. This will cause a syntax error.

Suggested change
options.RetryPolicy
options.RetryPolicy,

Copilot uses AI. Check for mistakes.
Copy link
Member

@vukelich vukelich Feb 20, 2026

Choose a reason for hiding this comment

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

Marking this as unresolved. @msalaman, do you have a local change to fix this syntax error not yet pushed?

alzimmermsft
alzimmermsft approved these changes Feb 19, 2026
View reviewed changes
Copy link
Contributor

@alzimmermsft alzimmermsft left a comment

Choose a reason for hiding this comment

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

Looks good! Just fix the two comments by copilot

@msalaman
Apply suggestions from code review
4d160f9 
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
vukelich
vukelich requested changes Feb 20, 2026
View reviewed changes
Copy link
Member

@vukelich vukelich left a comment

Choose a reason for hiding this comment

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

Reactivated one copilot find. Otherwise, ready to go.

servers/Azure.Mcp.Server/docs/new-command.md
options.Subscription!,
options.ResourceGroup,
options.RetryPolicy);
options.RetryPolicy
Copy link
Member

@vukelich vukelich Feb 20, 2026

Choose a reason for hiding this comment

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

Marking this as unresolved. @msalaman, do you have a local change to fix this syntax error not yet pushed?

@github-project-automation github-project-automation bot moved this from Untriaged to In Progress in Azure MCP Server Feb 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@vukelich vukelich vukelich requested changes

@alzimmermsft alzimmermsft alzimmermsft approved these changes

@jongio jongio Awaiting requested review from jongio jongio is a code owner

@xiangyan99 xiangyan99 Awaiting requested review from xiangyan99 xiangyan99 is a code owner

@qianwens qianwens Awaiting requested review from qianwens qianwens is a code owner

@hallipr hallipr Awaiting requested review from hallipr hallipr is a code owner automatically assigned from microsoft/azure-mcp

@saikoumudi saikoumudi Awaiting requested review from saikoumudi saikoumudi is a code owner automatically assigned from microsoft/azure-mcp

@sandeep-sen sandeep-sen Awaiting requested review from sandeep-sen sandeep-sen is a code owner automatically assigned from microsoft/azure-mcp

@JasonYeMSFT JasonYeMSFT Awaiting requested review from JasonYeMSFT JasonYeMSFT is a code owner automatically assigned from microsoft/azure-mcp

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

tools-Foundry Azure AI Foundry tools-Quota tools-Storage

Projects

Azure MCP Server
Status: In Progress

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@msalaman @vukelich @alzimmermsft