Monitoring expedite changes v4 #28533
Conversation
…e parameter - Added new parameter -Expedite to expedite the operation of a replicating server. - Updated documentation to include usage examples for the -Expedite parameter. - Modified existing examples to reflect new project and resource group names. - Enhanced output formatting in examples to provide clearer information. - Added tests for the new -Expedite functionality. - Updated ChangeLog to document the addition of the -Expedite feature.
| 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 the -Expedite parameter, which provides resource utilization analysis and recommendations to accelerate server migration operations. The enhancement adds a new parameter set GetByPrioritiseServer that analyzes shared resources among VMs and provides actionable suggestions for improving migration performance.
Key changes include:
- Added new parameter set and
-Expediteswitch parameter - Enhanced resource sharing analysis with detailed recommendations
- Improved output formatting and table generation
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Migrate/Migrate/help/Get-AzMigrateServerMigrationStatus.md | Added documentation for new -Expedite parameter and example usage |
| src/Migrate/Migrate/ChangeLog.md | Added changelog entry for the expedite functionality enhancement |
| src/Migrate/Migrate.Autorest/test/README.md | Minor documentation formatting update (backslash to forward slash) |
| src/Migrate/Migrate.Autorest/test/Get-AzMigrateServerMigrationStatus.Tests.ps1 | Added test stub for new GetByPrioritiseServer parameter set |
| src/Migrate/Migrate.Autorest/resources/README.md | Minor documentation formatting update (backslash to forward slash) |
| src/Migrate/Migrate.Autorest/examples/Get-AzMigrateServerMigrationStatus.md | Added example 4 showing expedite functionality with detailed output |
| src/Migrate/Migrate.Autorest/docs/Az.Migrate.md | Updated module GUID |
| src/Migrate/Migrate.Autorest/custom/README.md | Multiple documentation formatting updates (backslash to forward slash) |
| src/Migrate/Migrate.Autorest/custom/Get-AzMigrateServerMigrationStatus.ps1 | Major implementation changes adding expedite functionality, resource analysis, and recommendations |
| src/Migrate/Migrate.Autorest/custom/Az.Migrate.custom.psm1 | Documentation formatting update (backslash to forward slash) |
| src/Migrate/Migrate.Autorest/Properties/AssemblyInfo.cs | Version number changes |
|
|
||
| if ($ReplicationMigrationItem.MigrationState -match "MigrationInProgress" -and $ReplicationMigrationItem.migrationProgressPercentage -eq $null) { | ||
| return "FinalDeltaReplication Queued" | ||
| elseif ($ReplicationMigrationItem.MigrationState -match "InitialSeedingFailed") { |
There was a problem hiding this comment.
The state mapping logic should be consistent. Consider using -match for both conditions or -eq for both, rather than mixing comparison operators for similar string matching scenarios.
| elseif ($ReplicationMigrationItem.MigrationState -match "InitialSeedingFailed") { | |
| elseif ($ReplicationMigrationItem.MigrationState -eq "InitialSeedingFailed") { |
Copilot uses AI. Check for mistakes.
| if ($null -ne $Value) { | ||
| return "$Value %" | ||
| } else { | ||
| return "-" | ||
| } |
There was a problem hiding this comment.
The function name Add-Percent is misleading as it doesn't add anything - it formats a value with a percent symbol. Consider renaming to Format-AsPercent or ConvertTo-PercentString for clarity.
Copilot uses AI. Check for mistakes.
| } | ||
| } | ||
|
|
||
| function Add-MBps { |
There was a problem hiding this comment.
Similar to Add-Percent, the functions Add-MBps and Add-MB have misleading names as they format rather than add values. Consider renaming to Format-AsMBps and Format-AsMB respectively for better clarity.
Copilot uses AI. Check for mistakes.
| } | ||
| } | ||
|
|
||
| function Add-MB { |
There was a problem hiding this comment.
Similar to Add-Percent, the functions Add-MBps and Add-MB have misleading names as they format rather than add values. Consider renaming to Format-AsMBps and Format-AsMB respectively for better clarity.
Copilot uses AI. Check for mistakes.
| continue; | ||
| } | ||
| $replicationState = GetState -State $ReplicationMigrationItem.ProviderSpecificDetail.GatewayOperationDetailState -ReplicationMigrationItem $ReplicationMigrationItem | ||
| if ($replicationState -match "Failed" -or $replicationState -match "Completed" -or ($replicationState -notmatch "InProgress" -and $replicationState -notmatch "Queued")) { |
There was a problem hiding this comment.
The complex conditional logic for checking replication states could be simplified by defining valid states for expedite operations. Consider creating an array of valid states and using -in or -contains operators for better readability.
| if ($replicationState -match "Failed" -or $replicationState -match "Completed" -or ($replicationState -notmatch "InProgress" -and $replicationState -notmatch "Queued")) { | |
| $validExpediteStates = @("Queued", "InProgress") | |
| if (-not ($validExpediteStates -contains $replicationState)) { |
Copilot uses AI. Check for mistakes.
| if (-not $cols -or $cols.Count -eq 0) { $cols = $existingCols } | ||
|
|
||
| # Select columns explicitly and force Format-Table to render all of them (no truncation) | ||
| $vmMigrationStatusTable = $vmMigrationStatusTable | Select-Object -Property $cols | Format-Table -Property $cols -AutoSize -Wrap -Force | Out-String -Width 4096 |
There was a problem hiding this comment.
The hardcoded width value of 4096 should be defined as a named constant or configuration parameter to improve maintainability and make it easier to adjust if needed.
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.