[PS] migrate DiagnosticSetting in Monitor module to autorest v4 #28162
Conversation
| Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status. |
JoyerJin
changed the title
isra-fel
added
the
autorest v4 migration
There was a problem hiding this comment.
Pull Request Overview
This pull request adds Update cmdlets for Azure Diagnostic Settings and addresses breaking changes in the DiagnosticSetting AutoRest module. The key changes include:
- Added two new Update cmdlets:
Update-AzDiagnosticSettingandUpdate-AzSubscriptionDiagnosticSetting - Updated AutoRest configuration to use newer version and simplified model type names
- Regenerated module to fix breaking changes (property type changes in API models)
- Added exception handling for known breaking changes
- Updated tests to include Update cmdlet scenarios
- Added comprehensive documentation for new cmdlets
Reviewed Changes
Copilot reviewed 48 out of 48 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
tools/StaticAnalysis/Exceptions/Az.Monitor/BreakingChangeIssues.csv |
Adds exceptions for known breaking changes in property types |
src/Monitor/Monitor/help/*.md |
Updates help documentation for new Update cmdlets and fixes type references |
src/Monitor/Monitor/Az.Monitor.psd1 |
Adds new Update cmdlets to exported functions |
src/Monitor/Monitor.sln |
Updates project GUID for DiagnosticSetting.Autorest project |
src/Monitor/DiagnosticSetting.Autorest/test/*.ps1 |
Adds tests for Update cmdlets and updates existing tests |
src/Monitor/DiagnosticSetting.Autorest/docs/*.md |
Adds documentation for Update cmdlets |
src/Monitor/DiagnosticSetting.Autorest/README.md |
Updates AutoRest configuration to use newer directives |
src/Monitor/DiagnosticSetting.Autorest/custom/autogen-model-cmdlets/*.ps1 |
Updates model cmdlets with simplified type names |
| Update-AzSubscriptionDiagnosticSetting -Name settingname -WorkspaceId 'workspaceid' -Log $log | ||
| ``` | ||
|
|
||
| These command update diagnostic setting for current subscription. |
There was a problem hiding this comment.
Corrected grammar: 'command' should be 'commands' to match plural subject.
| These command update diagnostic setting for current subscription. | |
| These commands update diagnostic setting for current subscription. |
| } | ||
| function cleanupEnv() { | ||
| # Clean resources you create for testing | ||
| Remove-AzResourceGroup -Name $env.resourceGroupName |
There was a problem hiding this comment.
The cleanup function should handle cases where the resource group doesn't exist or deletion fails. Add error handling with -ErrorAction SilentlyContinue or wrap in try-catch to prevent test failures during cleanup.
| directive: | ||
| # Following is two common directive which are normally required in all the RPs | ||
| # 1. Remove the unexpanded parameter set | ||
| # 2. For New-* cmdlets, ViaIdentity is not required, so CreateViaIdentityExpanded is removed as well |
There was a problem hiding this comment.
This complex regex directive lacks a comment explaining its purpose. Based on CodingGuidelineID 1000002, all directives should have comments explaining the 'why'. Add a comment such as: '# Keep only Expanded, JsonFilePath, and JsonString parameter sets for Create and Update operations, removing unexpanded variants and CreateViaIdentityExpanded'.
| # 2. For New-* cmdlets, ViaIdentity is not required, so CreateViaIdentityExpanded is removed as well | |
| # 2. For New-* cmdlets, ViaIdentity is not required, so CreateViaIdentityExpanded is removed as well | |
| # Keep only Expanded, JsonFilePath, and JsonString parameter sets for Create and Update operations, | |
| # removing unexpanded variants and CreateViaIdentityExpanded to simplify the cmdlet surface and avoid unsupported scenarios. |
dolauli
added
Contains Breaking Change
|
To the author of the pull request, |
…e#28162) Co-authored-by: Yabo Hu <[email protected]>
Description
Mandatory Checklist
Please choose the target release of Azure PowerShell. (⚠️ Target release is a different concept from API readiness. Please click below links for details.)
Check this box to confirm: I have read the Submitting Changes section of
CONTRIBUTING.mdand reviewed the following information:ChangeLog.mdfile(s) appropriatelysrc/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.## Upcoming Releaseheader in the past tense.ChangeLog.mdif no new release is required, such as fixing test case only.