ManagedLustre: Use dash instead of underscore#1728
ManagedLustre: Use dash instead of underscore#1728conniey wants to merge 2 commits intomicrosoft:mainfrom
Conversation
conniey
requested review from
rebecca-makar and
wolfgang-desalvador
as code owners
conniey
requested review from
fanyang-mono,
hallipr,
joshfree,
msalaman,
saikoumudi,
sandeep-sen and
xiangyan99
There was a problem hiding this comment.
Pull request overview
This PR updates the Azure Managed Lustre tool to follow naming guidelines by changing CommandGroup names from underscore to dash separator. The changes replace blob_autoexport, blob_autoimport, and blob_import with blob-autoexport, blob-autoimport, and blob-import respectively. This aligns with the command design principles documented in new-command.md, which prohibit underscores in command group names. The PR is related to #1592, which adds validation to prevent future tools from using invalid naming conventions.
Changes:
- Updated three CommandGroup instantiations in ManagedLustreSetup.cs to use dashes instead of underscores
- This change affects the generated tool names (e.g.,
managedlustre_fs_blob_autoexport_createbecomesmanagedlustre_fs_blob-autoexport_create) - Aligns with established naming guidelines that require concatenated or dash-separated command group names
| @@ -89,7 +89,7 @@ public CommandGroup RegisterCommands(IServiceProvider serviceProvider) | |||
| var autoexportJobDelete = serviceProvider.GetRequiredService<AutoexportJobDeleteCommand>(); | |||
| autoexportJob.AddCommand(autoexportJobDelete.Name, autoexportJobDelete); | |||
|
|
|||
| var autoimportJob = new CommandGroup("blob_autoimport", "Autoimport job operations for Azure Managed Lustre - Commands for creating jobs to import data from blob storage to the filesystem."); | |||
| var autoimportJob = new CommandGroup("blob-autoimport", "Autoimport job operations for Azure Managed Lustre - Commands for creating jobs to import data from blob storage to the filesystem."); | |||
| fileSystem.AddSubGroup(autoimportJob); | |||
|
|
|||
| var autoimportJobCreate = serviceProvider.GetRequiredService<AutoimportJobCreateCommand>(); | |||
| @@ -104,7 +104,7 @@ public CommandGroup RegisterCommands(IServiceProvider serviceProvider) | |||
| var autoimportJobDelete = serviceProvider.GetRequiredService<AutoimportJobDeleteCommand>(); | |||
| autoimportJob.AddCommand(autoimportJobDelete.Name, autoimportJobDelete); | |||
|
|
|||
| var blobImport = new CommandGroup("blob_import", "One-time blob import operations for Azure Managed Lustre - Commands for creating jobs to perform one-time import of data from blob storage to the filesystem."); | |||
| var blobImport = new CommandGroup("blob-import", "One-time blob import operations for Azure Managed Lustre - Commands for creating jobs to perform one-time import of data from blob storage to the filesystem."); | |||
There was a problem hiding this comment.
The CommandGroup name changes from underscore to dash will change the full tool names. For example, blob_autoexport becoming blob-autoexport means the tool managedlustre_fs_blob_autoexport_create will become managedlustre_fs_blob-autoexport_create. However, the test files still reference the old naming convention with underscores (e.g., managedlustre_fs_blob_autoexport_create in ManagedLustreCommandTests.cs lines 381, 397, 423, 442, 459, and similar for autoimport and import). These test references need to be updated to use dashes instead of underscores in the command group portion of the tool name.
| @@ -89,7 +89,7 @@ public CommandGroup RegisterCommands(IServiceProvider serviceProvider) | |||
| var autoexportJobDelete = serviceProvider.GetRequiredService<AutoexportJobDeleteCommand>(); | |||
| autoexportJob.AddCommand(autoexportJobDelete.Name, autoexportJobDelete); | |||
|
|
|||
| var autoimportJob = new CommandGroup("blob_autoimport", "Autoimport job operations for Azure Managed Lustre - Commands for creating jobs to import data from blob storage to the filesystem."); | |||
| var autoimportJob = new CommandGroup("blob-autoimport", "Autoimport job operations for Azure Managed Lustre - Commands for creating jobs to import data from blob storage to the filesystem."); | |||
| fileSystem.AddSubGroup(autoimportJob); | |||
|
|
|||
| var autoimportJobCreate = serviceProvider.GetRequiredService<AutoimportJobCreateCommand>(); | |||
| @@ -104,7 +104,7 @@ public CommandGroup RegisterCommands(IServiceProvider serviceProvider) | |||
| var autoimportJobDelete = serviceProvider.GetRequiredService<AutoimportJobDeleteCommand>(); | |||
| autoimportJob.AddCommand(autoimportJobDelete.Name, autoimportJobDelete); | |||
|
|
|||
| var blobImport = new CommandGroup("blob_import", "One-time blob import operations for Azure Managed Lustre - Commands for creating jobs to perform one-time import of data from blob storage to the filesystem."); | |||
| var blobImport = new CommandGroup("blob-import", "One-time blob import operations for Azure Managed Lustre - Commands for creating jobs to perform one-time import of data from blob storage to the filesystem."); | |||
There was a problem hiding this comment.
The documentation in servers/Azure.Mcp.Server/CHANGELOG.md (lines 209-212) and servers/Azure.Mcp.Server/vscode/CHANGELOG.md (lines 156-159) still references the old tool names with underscores (e.g., managedlustre_fs_blob_autoexport_create). These need to be updated to use dashes in the command group name (e.g., managedlustre_fs_blob-autoexport_create) to match the code changes.
| @@ -89,7 +89,7 @@ public CommandGroup RegisterCommands(IServiceProvider serviceProvider) | |||
| var autoexportJobDelete = serviceProvider.GetRequiredService<AutoexportJobDeleteCommand>(); | |||
| autoexportJob.AddCommand(autoexportJobDelete.Name, autoexportJobDelete); | |||
|
|
|||
| var autoimportJob = new CommandGroup("blob_autoimport", "Autoimport job operations for Azure Managed Lustre - Commands for creating jobs to import data from blob storage to the filesystem."); | |||
| var autoimportJob = new CommandGroup("blob-autoimport", "Autoimport job operations for Azure Managed Lustre - Commands for creating jobs to import data from blob storage to the filesystem."); | |||
| fileSystem.AddSubGroup(autoimportJob); | |||
|
|
|||
| var autoimportJobCreate = serviceProvider.GetRequiredService<AutoimportJobCreateCommand>(); | |||
| @@ -104,7 +104,7 @@ public CommandGroup RegisterCommands(IServiceProvider serviceProvider) | |||
| var autoimportJobDelete = serviceProvider.GetRequiredService<AutoimportJobDeleteCommand>(); | |||
| autoimportJob.AddCommand(autoimportJobDelete.Name, autoimportJobDelete); | |||
|
|
|||
| var blobImport = new CommandGroup("blob_import", "One-time blob import operations for Azure Managed Lustre - Commands for creating jobs to perform one-time import of data from blob storage to the filesystem."); | |||
| var blobImport = new CommandGroup("blob-import", "One-time blob import operations for Azure Managed Lustre - Commands for creating jobs to perform one-time import of data from blob storage to the filesystem."); | |||
There was a problem hiding this comment.
The test prompts documentation in servers/Azure.Mcp.Server/docs/e2eTestPrompts.md (lines 490-498) still references the old tool names with underscores (e.g., managedlustre_fs_blob_autoexport_create, managedlustre_fs_blob_autoimport_create, managedlustre_fs_blob_import_create). These need to be updated to use dashes in the command group name (e.g., managedlustre_fs_blob-autoexport_create) to match the code changes.
| @@ -89,7 +89,7 @@ public CommandGroup RegisterCommands(IServiceProvider serviceProvider) | |||
| var autoexportJobDelete = serviceProvider.GetRequiredService<AutoexportJobDeleteCommand>(); | |||
| autoexportJob.AddCommand(autoexportJobDelete.Name, autoexportJobDelete); | |||
|
|
|||
| var autoimportJob = new CommandGroup("blob_autoimport", "Autoimport job operations for Azure Managed Lustre - Commands for creating jobs to import data from blob storage to the filesystem."); | |||
| var autoimportJob = new CommandGroup("blob-autoimport", "Autoimport job operations for Azure Managed Lustre - Commands for creating jobs to import data from blob storage to the filesystem."); | |||
| fileSystem.AddSubGroup(autoimportJob); | |||
|
|
|||
| var autoimportJobCreate = serviceProvider.GetRequiredService<AutoimportJobCreateCommand>(); | |||
| @@ -104,7 +104,7 @@ public CommandGroup RegisterCommands(IServiceProvider serviceProvider) | |||
| var autoimportJobDelete = serviceProvider.GetRequiredService<AutoimportJobDeleteCommand>(); | |||
| autoimportJob.AddCommand(autoimportJobDelete.Name, autoimportJobDelete); | |||
|
|
|||
| var blobImport = new CommandGroup("blob_import", "One-time blob import operations for Azure Managed Lustre - Commands for creating jobs to perform one-time import of data from blob storage to the filesystem."); | |||
| var blobImport = new CommandGroup("blob-import", "One-time blob import operations for Azure Managed Lustre - Commands for creating jobs to perform one-time import of data from blob storage to the filesystem."); | |||
There was a problem hiding this comment.
The consolidated-tools.json file in core/Microsoft.Mcp.Core/src/Areas/Server/Resources/consolidated-tools.json still references the old tool names with underscores (lines 1461, 1540, 1608, 1609, and similar for autoimport and import). According to the PR checklist, this file needs to be updated for tools with new names. All references to managedlustre_fs_blob_autoexport_*, managedlustre_fs_blob_autoimport_*, and managedlustre_fs_blob_import_* need to be updated to use dashes (e.g., managedlustre_fs_blob-autoexport_*).
| @@ -89,7 +89,7 @@ public CommandGroup RegisterCommands(IServiceProvider serviceProvider) | |||
| var autoexportJobDelete = serviceProvider.GetRequiredService<AutoexportJobDeleteCommand>(); | |||
| autoexportJob.AddCommand(autoexportJobDelete.Name, autoexportJobDelete); | |||
|
|
|||
| var autoimportJob = new CommandGroup("blob_autoimport", "Autoimport job operations for Azure Managed Lustre - Commands for creating jobs to import data from blob storage to the filesystem."); | |||
| var autoimportJob = new CommandGroup("blob-autoimport", "Autoimport job operations for Azure Managed Lustre - Commands for creating jobs to import data from blob storage to the filesystem."); | |||
| fileSystem.AddSubGroup(autoimportJob); | |||
|
|
|||
| var autoimportJobCreate = serviceProvider.GetRequiredService<AutoimportJobCreateCommand>(); | |||
| @@ -104,7 +104,7 @@ public CommandGroup RegisterCommands(IServiceProvider serviceProvider) | |||
| var autoimportJobDelete = serviceProvider.GetRequiredService<AutoimportJobDeleteCommand>(); | |||
| autoimportJob.AddCommand(autoimportJobDelete.Name, autoimportJobDelete); | |||
|
|
|||
| var blobImport = new CommandGroup("blob_import", "One-time blob import operations for Azure Managed Lustre - Commands for creating jobs to perform one-time import of data from blob storage to the filesystem."); | |||
| var blobImport = new CommandGroup("blob-import", "One-time blob import operations for Azure Managed Lustre - Commands for creating jobs to perform one-time import of data from blob storage to the filesystem."); | |||
There was a problem hiding this comment.
According to the PR checklist, .\eng\scripts\Update-AzCommandsMetadata.ps1 should be run to update tool metadata in azmcp-commands.md. This script needs to be executed to ensure the metadata and tool references are correctly updated after changing the CommandGroup names from underscores to dashes. The script should handle updating any internal tool name references if needed.
| autoimportJob.AddCommand(autoimportJobDelete.Name, autoimportJobDelete); | ||
|
|
||
| var blobImport = new CommandGroup("blob_import", "One-time blob import operations for Azure Managed Lustre - Commands for creating jobs to perform one-time import of data from blob storage to the filesystem."); | ||
| var blobImport = new CommandGroup("blob-import", "One-time blob import operations for Azure Managed Lustre - Commands for creating jobs to perform one-time import of data from blob storage to the filesystem."); |
There was a problem hiding this comment.
According to the PR checklist, the CHANGELOG.md should be updated for product changes. Since this change modifies the tool names (from managedlustre_fs_blob_autoexport_* to managedlustre_fs_blob-autoexport_*, etc.), it's a breaking change that should be documented in the "Breaking Changes" section of servers/Azure.Mcp.Server/CHANGELOG.md under the "2.0.0-beta.20 (Unreleased)" section. Users relying on the old tool names will need to update their code.
| var blobImport = new CommandGroup("blob-import", "One-time blob import operations for Azure Managed Lustre - Commands for creating jobs to perform one-time import of data from blob storage to the filesystem."); | |
| var blobImport = new CommandGroup("blob_import", "One-time blob import operations for Azure Managed Lustre - Commands for creating jobs to perform one-time import of data from blob storage to the filesystem."); |
2 participants
What does this PR do?
Related #1592
GitHub issue number?
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