Enhance Get-AzMigrateServerMigrationStatus cmdlet to support -Expedit… #28528
Conversation
…e parameter - Added -Expedite parameter to expedite the operation of a replicating server. - Updated documentation to include new parameter and usage examples. - Modified examples to reflect new project and resource group names. - Enhanced output formatting in examples for clarity. - Added test case for GetByPrioritiseServer functionality.
| Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status. |
|
Thank you for your contribution @ankitbaluni123! We will review the pull request and get back to you soon. |
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the Get-AzMigrateServerMigrationStatus cmdlet by adding a new -Expedite parameter that provides detailed resource utilization analysis and optimization recommendations for server migration operations. The update also includes comprehensive documentation updates with new examples and improved output formatting.
- Added
-Expediteparameter with a new parameter setGetByPrioritiseServerfor migration optimization analysis - Enhanced output formatting to include ESXiHost information and improved table display
- Updated documentation with new examples demonstrating the expedite functionality
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Migrate/Migrate/help/Get-AzMigrateServerMigrationStatus.md | Added documentation for new -Expedite parameter and updated examples with new project/resource group names |
| src/Migrate/Migrate/ChangeLog.md | Added changelog entry for the new -Expedite parameter support |
| src/Migrate/Migrate.Autorest/test/Get-AzMigrateServerMigrationStatus.Tests.ps1 | Added test case for the new GetByPrioritiseServer parameter set |
| src/Migrate/Migrate.Autorest/custom/Get-AzMigrateServerMigrationStatus.ps1 | Core implementation of the -Expedite functionality with resource analysis and recommendations |
| src/Migrate/Migrate.Autorest/examples/Get-AzMigrateServerMigrationStatus.md | Updated examples with new parameter usage and output formatting |
| src/Migrate/Migrate.Autorest/docs/Get-AzMigrateServerMigrationStatus.md | Mirror of help file updates for consistent documentation |
| if ($ReplicationMigrationItem.MigrationState -eq "MigrationFailed") { | ||
| return "Migration Failed" | ||
| } |
There was a problem hiding this comment.
The logic flow is incorrect. The function checks for MigrationFailed state but then immediately continues to check for InitialSeedingFailed without returning. This could lead to the wrong state being returned for failed migrations.
Copilot uses AI. Check for mistakes.
| if ($Value -ne $null) { | ||
| return "$Value %" | ||
| } else { | ||
| return "-" | ||
| } |
There was a problem hiding this comment.
The condition $Value -ne $null will return percentage for zero values, which may not be the intended behavior. Consider checking for both null and zero values if zero should display as '-'.
Copilot uses AI. Check for mistakes.
| if ($Capacity -eq "-" -or $Capacity -eq 0 -or $Capacity -eq $null) { | ||
| return "-" | ||
| } | ||
| if ($Utilization -eq "-" -or $Utilization -eq $null) { |
There was a problem hiding this comment.
Comparing a numeric variable $Capacity to the string '-' will never be true. This comparison should be removed or the parameter type should be clarified.
| if ($Capacity -eq "-" -or $Capacity -eq 0 -or $Capacity -eq $null) { | |
| return "-" | |
| } | |
| if ($Utilization -eq "-" -or $Utilization -eq $null) { | |
| if ($Capacity -eq 0 -or $Capacity -eq $null) { | |
| return "-" | |
| } | |
| if ($Utilization -eq $null) { |
Copilot uses AI. Check for mistakes.
| $row5["Capacity"] = $diskCapacity ?? "-" | ||
| $row5["Utilization for server migrations"] = $diskProcessUtil ?? "-" |
There was a problem hiding this comment.
The null-coalescing operator ?? is not available in PowerShell 5.1. Use if ($diskCapacity -ne $null) { $diskCapacity } else { '-' } syntax for compatibility.
Copilot uses AI. Check for mistakes.
| $datastoreName = $ds.datastoreName ?? "-" | ||
| $row["Resource"] = "Datastore '$datastoreName' Snapshot Count" | ||
| $row["Capacity"] = $ds.totalSnapshotsSupported ?? "-" | ||
| $row["Utilization for server migrations"] = $ds.totalSnapshotsCreated ?? "-" |
There was a problem hiding this comment.
Multiple uses of the null-coalescing operator ?? which is not available in PowerShell 5.1. Replace with conditional expressions for compatibility.
Copilot uses AI. Check for mistakes.
|
Created new PR based on the additonal suggestion of separate chages requirement. |
…e parameter
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.