Monitoring expedite changes v1 #28530
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. |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for the -Expedite parameter to the Get-AzMigrateServerMigrationStatus cmdlet, enabling users to view resource utilization and recommendations for expediting server migration operations.
Key Changes
- Added new
-Expediteparameter withGetByPrioritiseServerparameter set for analyzing migration bottlenecks - Enhanced output to include ESXi host information and detailed resource sharing analysis
- Implemented resource utilization monitoring with capacity, utilization metrics, and status assessment
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 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 current output format |
src/Migrate/Migrate/ChangeLog.md |
Added changelog entry for the -Expedite parameter enhancement |
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 |
Implemented core -Expedite functionality with resource analysis and recommendations |
src/Migrate/Migrate.Autorest/examples/Get-AzMigrateServerMigrationStatus.md |
Updated examples to reflect new parameter and enhanced output format |
| Multiple README.md files | Updated relative path references from .. to ../ for consistency |
src/Migrate/Migrate.Autorest/Properties/AssemblyInfo.cs |
Version numbers updated to development version |
| if ($null -ne $Value) { | ||
| return "$Value %" |
There was a problem hiding this comment.
The condition $null -ne $Value should also check for zero values. A zero percentage is a valid value that should be displayed as '0 %', not '-'. Consider changing to if ($null -ne $Value -and $Value -ne 0) { return \"$Value %\" } elseif ($Value -eq 0) { return \"0 %\" } else { return \"-\" }
| if ($null -ne $Value) { | |
| return "$Value %" | |
| if ($null -ne $Value -and $Value -ne 0) { | |
| return "$Value %" | |
| } elseif ($Value -eq 0) { | |
| return "0 %" |
Copilot uses AI. Check for mistakes.
| foreach ($rec in $recommendations) { | ||
| $op = $output.Add("$rec") |
There was a problem hiding this comment.
[nitpick] The recommendations are numbered in the help documentation examples (e.g., '1. CPU is At capacity...') but the code doesn't add numbering. Consider adding a counter to number the recommendations for consistency with the documented output format.
| foreach ($rec in $recommendations) { | |
| $op = $output.Add("$rec") | |
| $counter = 1 | |
| foreach ($rec in $recommendations) { | |
| $op = $output.Add("$counter. $rec") | |
| $counter++ |
Copilot uses AI. Check for mistakes.
| } | ||
| if ($null -eq $Utilization) { | ||
| return "-" | ||
| } |
There was a problem hiding this comment.
[nitpick] The special case handling for CPU utilization of 0 seems inconsistent with other resource types. Consider documenting why CPU utilization of 0 should return '-' rather than 'Underutilized' like other resources.
| } | |
| } | |
| # For CPU resources, a utilization of 0 may indicate that no data was collected or the metric is not applicable. | |
| # Therefore, we return "-" instead of "Underutilized" to distinguish this case from true underutilization. |
Copilot uses AI. Check for mistakes.
| } else { | ||
| $row2["Capacity"] = "-" | ||
| } | ||
| $row2["Utilization for server migrations"] = Add-Percent -Value $cpuProcessUtil |
There was a problem hiding this comment.
There's an inconsistency in the CPU status calculation. Line 698 uses $cpuProcessUtil for process utilization display, but line 700 uses $cpuTotalUtil for status calculation. The status should be based on total utilization, but this should be documented or the logic should be consistent.
| $row2["Utilization for server migrations"] = Add-Percent -Value $cpuProcessUtil | |
| $row2["Utilization for server migrations"] = Add-Percent -Value $cpuTotalUtil |
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.