Enhance Get-AzMigrateServerMigrationStatus cmdlet #28535
Conversation
- Added support for the -Expedite parameter to expedite server migration operations. - Updated documentation to include new usage examples for the -Expedite parameter. - Modified existing examples to reflect updated project and resource group names. - Enhanced output formatting in examples for better clarity. - Added test case for the new -Expedite functionality. - Updated ChangeLog to reflect the addition of the -Expedite parameter.
| 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 support for an -Expedite parameter that provides resource utilization analysis and recommendations to optimize server migration performance. The enhancement focuses on identifying resource bottlenecks and offering actionable suggestions to expedite migrations.
Key Changes:
- Added
-Expediteparameter with a new parameter setGetByPrioritiseServer - Implemented comprehensive resource utilization analysis functionality
- Enhanced output formatting to include ESXi host information across all parameter sets
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/Migrate/Migrate/ChangeLog.md |
Added changelog entry documenting the -Expedite parameter enhancement |
src/Migrate/Migrate.Autorest/test/Get-AzMigrateServerMigrationStatus.Tests.ps1 |
Added placeholder test case for the new GetByPrioritiseServer parameter set |
src/Migrate/Migrate.Autorest/examples/Get-AzMigrateServerMigrationStatus.md |
Updated examples with new project/resource group names and added comprehensive Example 4 showcasing -Expedite functionality |
src/Migrate/Migrate.Autorest/docs/Get-AzMigrateServerMigrationStatus.md |
Updated documentation to include -Expedite parameter syntax, description, and detailed usage examples |
src/Migrate/Migrate.Autorest/custom/Get-AzMigrateServerMigrationStatus.ps1 |
Implemented core -Expedite functionality including resource sharing analysis, utilization monitoring, and recommendation engine |
| if ($ReplicationMigrationItem.MigrationState -eq "MigrationFailed") { | ||
| return "Migration Failed" | ||
| } | ||
|
|
||
| if ($ReplicationMigrationItem.MigrationState -match "MigrationInProgress" -and $ReplicationMigrationItem.migrationProgressPercentage -eq $null) { | ||
| return "FinalDeltaReplication Queued" | ||
| elseif ($ReplicationMigrationItem.MigrationState -match "InitialSeedingFailed") { | ||
| return "InitialReplication Failed" | ||
| } | ||
|
|
||
| if ($ReplicationMigrationItem.MigrationState -eq "MigrationSucceeded") { | ||
| return "Migration Completed" | ||
| if ([string]::IsNullOrEmpty($State)) { | ||
| return $ReplicationMigrationItem.MigrationState | ||
| } |
There was a problem hiding this comment.
The logic flow is incorrect. The function checks for 'MigrationFailed' and 'InitialSeedingFailed' states but then has an unreachable section that checks if State is null/empty. If State is passed as null/empty, the MigrationState checks will execute, but the null check at line 200 becomes unreachable when the previous conditions are met. Consider restructuring the logic to handle the State parameter first.
Copilot uses AI. Check for mistakes.
| if ($null -ne $Value) { | ||
| return "$Value %" | ||
| } else { | ||
| return "-" | ||
| } |
There was a problem hiding this comment.
The condition should also check for zero values. A progress value of 0 is valid and should be displayed as '0 %' rather than '-'. Consider changing the condition to check if Value is not null and not treating 0 as empty.
Copilot uses AI. Check for mistakes.
| $ramCapacityVal = 0 | ||
| if ($row["Capacity"] -match "(\d+(\.\d+)?)") { $ramCapacityVal = [double]$matches[1] } | ||
| if ($ramCapacityVal -lt 32768 -and $ramCapacityVal -gt 0) { | ||
| $recommendations += "Consider increasing the appliance RAM to improve migration performance and support higher workloads." | ||
| } |
There was a problem hiding this comment.
The magic number 32768 (MB) should be defined as a named constant to improve code maintainability and make the RAM threshold more explicit. Consider defining this as a constant like $RECOMMENDED_MIN_RAM_MB = 32768 at the beginning of the function.
Copilot uses AI. Check for mistakes.
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.