[ChangeSafety] Fix base calls in GetDynamicParameters#29222
Open
[ChangeSafety] Fix base calls in GetDynamicParameters#29222
Conversation
| Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status. |
notyashhh
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates several cmdlets’ GetDynamicParameters() implementations to preserve and return dynamic parameters provided by base cmdlet classes, instead of returning null or always starting from a new empty dictionary. This helps ensure inherited dynamic parameters (from AzureRMCmdlet/AzurePSCmdlet/module base cmdlets) continue to flow through when a cmdlet doesn’t add its own dynamic parameters.
Changes:
- Return
base.GetDynamicParameters()instead ofnullwhen no custom dynamic parameters are needed (Storage, IoTHub, CosmosDB). - Seed
RuntimeDefinedParameterDictionaryinstances frombase.GetDynamicParameters()before adding cmdlet-specific runtime parameters (Network, Resources, DataMigration, Accounts config). - Minor whitespace-only change in
TestDeploymentCmdletBase.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Storage/Storage/File/Cmdlet/SetAzureStorageFileContent.cs | Returns base dynamic parameters on non-Windows instead of null. |
| src/Storage/Storage/File/Cmdlet/GetAzureStorageFileContent.cs | Returns base dynamic parameters on non-Windows instead of null. |
| src/Resources/ResourceManager/Implementation/CmdletBase/TestDeploymentCmdletBase.cs | Whitespace-only change in method spacing. |
| src/Resources/ResourceManager/Implementation/CmdletBase/DeploymentStacksCreateCmdletBase.cs | Seeds dynamic parameter dictionary from base.GetDynamicParameters(). |
| src/Resources/ResourceManager/Implementation/CmdletBase/DeploymentCmdletBase.cs | Seeds dynamic parameter dictionary from base.GetDynamicParameters(). |
| src/Network/Network/PrivateLinkService/PrivateLinkResource/GetAzurePrivateLinkResourceCommand.cs | Seeds runtime parameter dictionary from base before adding provider-specific parameter. |
| src/Network/Network/PrivateLinkService/PrivateEndpointConnection/PrivateEndpointConnectionBaseCmdlet.cs | Seeds runtime parameter dictionary from base before adding provider-specific parameter. |
| src/IotHub/IotHub/IotHub/Routing/AddAzureRmIotHubRoutingEndpoint.cs | Returns base dynamic parameters when endpoint type doesn’t require custom dynamic params. |
| src/IotHub/IotHub/IotHub/DataPlane/Module/SetAzIotHubModule.cs | Returns base dynamic parameters when auth method doesn’t require custom dynamic params. |
| src/IotHub/IotHub/IotHub/DataPlane/Module/AddAzIotHubModule.cs | Returns base dynamic parameters when auth method doesn’t require custom dynamic params. |
| src/IotHub/IotHub/IotHub/DataPlane/Device/SetAzIotHubDevice.cs | Returns base dynamic parameters when auth method doesn’t require custom dynamic params. |
| src/IotHub/IotHub/IotHub/DataPlane/Device/AddAzIotHubDevice.cs | Returns base dynamic parameters when auth method doesn’t require custom dynamic params. |
| src/DataMigration/DataMigration/Cmdlets/NewDataMigrationTask.cs | Merges base dynamic parameters with task-type-specific runtime params and returns combined dictionary. |
| src/DataMigration/DataMigration/Cmdlets/NewConnectionInfoCmdlet.cs | Merges base dynamic parameters with server-type-specific runtime params and returns combined dictionary. |
| src/DataMigration/DataMigration/Cmdlets/InvokeDataMigrationCommand.cs | Merges base dynamic parameters with command-type-specific runtime params and returns combined dictionary. |
| src/CosmosDB/CosmosDB/CosmosDBAccount/NewAzCosmosDBAccount.cs | Returns base dynamic parameters when PIT restore dynamic params aren’t used. |
| src/Accounts/Accounts/Config/ConfigCommandBase.cs | Preserves base-class dynamic parameters before adding config-key runtime params. |
| return base.GetDynamicParameters(); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Line contains trailing whitespace (blank line with spaces). Please remove the extra spaces to avoid unnecessary diffs and formatting noise.
Suggested change
isra-fel
previously approved these changes
Mar 6, 2026
Member
isra-fel
left a comment
isra-fel
left a comment
There was a problem hiding this comment.
Consider updating the ChangeLog.md file for these modules cause otherwise the change safety work still won't be reflected on them.
This could of cause be defered to the same release of the wave 1 work of change safety. So I'll approve the PR first and let you decide @notyashhh
Co-authored-by: Joselyn Narvaez <jnarvaez@microsoft.com>
Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Lauren Kam <laurenkam@microsoft.com>
…dd-AzVMDataDisk (#29194) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: audreyttt <225061541+audreyttt@users.noreply.github.com>
Co-authored-by: Yeming Liu <yeliu@microsoft.com>
…table (#28902) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Yash <55773468+notyashhh@users.noreply.github.com> Co-authored-by: Yeming Liu <11371776+isra-fel@users.noreply.github.com>
…s in `New-DevTSPModule` (#29213)
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
#29200) Co-authored-by: Raj Kamal <kamalraj@microsoft.com>
Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…hedulers (#29218) Co-authored-by: Bernd Verst <beverst@microsoft.com>
Co-authored-by: Carlos Hinojosa Cavada <carlosh@microsoft.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…ipeline example (#29294) Co-authored-by: coder999999999 <coder999999999@users.noreply.github.com> Co-authored-by: Nori Zhang <norizhang@microsoft.com>
notyashhh
requested review from
NoriZC,
VeryEarly,
audreyttt,
haagha,
ookoka and
teresaritorto
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 3431 changed files in this pull request and generated 15 comments.
Comments suppressed due to low confidence (1)
documentation/breaking-changes/upcoming-breaking-changes.md:1
- This entry is internally inconsistent: it states the output type is 'bool' but then describes deprecated/added properties on the output type. If the cmdlet returns a boolean, the property-change bullets should be removed; if the cmdlet returns a model type, the output type line should be corrected accordingly.
# Upcoming breaking changes in Azure PowerShell
Member
Author
|
@isra-fel Requesting a review again. |
Member
Author
|
Related change in powershell common: Azure/azure-powershell-common#449 |
...Network/Network/PrivateLinkService/PrivateLinkResource/GetAzurePrivateLinkResourceCommand.cs
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes
GetDynamicParameters()overrides across 6 modules to properly chain tobase.GetDynamicParameters(). This ensures dynamic parameters from base classes are inherited correctly — required for the upcoming Change Safety parameters (-AcquirePolicyToken,-ChangeReference) fromazure-powershell-common.Common PR: Azure/azure-powershell-common#449
Modules changed
ConfigCommandBase.csNewAzCosmosDBAccount.csbase.GetDynamicParameters()instead of returning nullInvokeDataMigrationCommand.cs,NewConnectionInfoCmdlet.cs,NewDataMigrationTask.csAddAzIotHubDevice.cs,SetAzIotHubDevice.cs,AddAzIotHubModule.cs,SetAzIotHubModule.cs,AddAzureRmIotHubRoutingEndpoint.csbase.GetDynamicParameters()and return base result in default casePrivateEndpointConnectionBaseCmdlet.cs,GetAzurePrivateLinkResourceCommand.csbase.GetDynamicParameters()instead of creating empty dictionaryDeploymentCmdletBase.cs,DeploymentStacksCreateCmdletBase.csImpact
azure-powershell-commonrepoMandatory 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.