Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug 28957665: Fixed psObject List Enumeration #25781

Merged

Conversation

notyashhh
Copy link
Member

@notyashhh notyashhh commented Aug 7, 2024

Problem Description:

  • The Update-AzMaintenanceConfiguration command was failing with the error:
    Cannot convert 'Microsoft.Azure.Commands.Maintenance.Models.PSMaintenanceConfiguration' to the type 'Microsoft.Azure.Commands.Maintenance.Models.PSMaintenanceConfiguration' required by parameter 'Configuration'. Specified method is not supported.

This issue occurred when Get-AzMaintenanceConfiguration was called without the ResourceGroupName parameter, causing the returned object to be of type List, instead of a single PSMaintenanceConfiguration.

Fix Implemented:

  • Modified the Update-AzMaintenanceConfiguration command to handle both cases where Get-AzMaintenanceConfiguration returns a list of configurations or a single configuration object.
  • Ensured that the command properly handles and converts the list of configurations into individual PSMaintenanceConfiguration objects when required.

Verification:

  • Verified that Update-AzMaintenanceConfiguration works correctly with both single and multiple maintenance configurations.
    Added test cases to cover both scenarios ensuring the command operates as expected.

Issue Addressed:

Testing:

  • Ran existing tests to confirm no regressions.
  • Added new test cases for scenarios involving single and multiple maintenance configurations.

Impact:

  • This fix ensures that users can now successfully update maintenance configurations regardless of whether they specify the ResourceGroupName parameter when calling Get-AzMaintenanceConfiguration.

Mandatory Checklist

  • SHOULD update ChangeLog.md file(s) appropriately
    • For SDK-based development mode, update src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.
      • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header in the past tense.
    • For autorest-based development mode, include the changelog in the PR description.
    • Should not change ChangeLog.md if no new release is required, such as fixing test case only.
  • SHOULD regenerate markdown help files if there is cmdlet API change. Instruction
  • SHOULD have proper test coverage for changes in pull request.
  • SHOULD NOT adjust version of module manually in pull request

Copy link

azure-client-tools-bot-prd bot commented Aug 7, 2024

️✔️Az.Accounts
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Az.Maintenance
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Breaking Change Check
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Signature Check
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Help File Existence Check
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️File Change Check
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️UX Metadata Check
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Test
️✔️PowerShell Core - Linux
️✔️PowerShell Core - MacOS
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows

@BethanyZhou
Copy link
Contributor

Hi @notyashhh,

  • Could you link this PR with the original GitHub issue so that the issue will be closed automatically once the PR gets merge? BTW, ADO item, which is an internal way to track issue, should not be exposed in our public GitHub repo.
  • Please check the mandatory checklist in the PR board.

src/Maintenance/Maintenance/ChangeLog.md Outdated Show resolved Hide resolved
src/Maintenance/Maintenance/ChangeLog.md Outdated Show resolved Hide resolved
src/Maintenance/Maintenance/ChangeLog.md Outdated Show resolved Hide resolved
@dolauli
Copy link
Contributor

dolauli commented Aug 8, 2024

/azp run azure-powershell - security-tools

Copy link
Contributor

Azure Pipelines successfully started running 1 pipeline(s).

@dolauli dolauli self-assigned this Aug 8, 2024
@dolauli
Copy link
Contributor

dolauli commented Aug 9, 2024

/azp run azure-powershell - security-tools

Copy link
Contributor

Azure Pipelines successfully started running 1 pipeline(s).

@notyashhh
Copy link
Member Author

/azp run azure-powershell - security-tools

Copy link
Contributor

Commenter does not have sufficient privileges for PR 25781 in repo Azure/azure-powershell

1 similar comment
Copy link
Contributor

Commenter does not have sufficient privileges for PR 25781 in repo Azure/azure-powershell

@notyashhh notyashhh self-assigned this Aug 9, 2024
@BethanyZhou
Copy link
Contributor

I think maybe we need two configurations in the test cases to check if the list will be unpacking as single enumeration

Copy link

github-actions bot commented Aug 9, 2024

This PR was labeled "needs-revision" because it has unresolved review comments or CI failures.
Please resolve all open review comments and make sure all CI checks are green. Refer to our guide to troubleshoot common CI failures.

@BethanyZhou
Copy link
Contributor

@notyashhh, if your PR is ready for reviewing again, add needs-review label.

Yash Patil added 2 commits August 12, 2024 15:10
…ub.com:notyashhh/azure-powershell into fix/28957665-Update-AzMaintenanceConfiguration
BethanyZhou
BethanyZhou previously approved these changes Aug 13, 2024
Copy link
Contributor

@BethanyZhou BethanyZhou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job. Looks good to me.

@BethanyZhou
Copy link
Contributor

/azp run azure-powershell - security-tools

Copy link
Contributor

Azure Pipelines successfully started running 1 pipeline(s).

@BethanyZhou
Copy link
Contributor

/azp run azure-powershell - security-tools

Copy link
Contributor

Azure Pipelines successfully started running 1 pipeline(s).

Fix the security pipeline failure.
@dolauli
Copy link
Contributor

dolauli commented Aug 13, 2024

/azp run azure-powershell - security-tools

Copy link
Contributor

Azure Pipelines successfully started running 1 pipeline(s).

@dolauli dolauli merged commit 2f75b42 into Azure:main Aug 13, 2024
12 checks passed
github-actions bot pushed a commit that referenced this pull request Aug 13, 2024
* Fixed psObject List Enumeration

* Added test case: UpdateConfig cmdlet

* Updated Changelog

* Additioanl Test Case + Updated Changelog

* Added complex test case: Multiple Configs

* Update NuGet.Config

Fix the security pipeline failure.

* revert nuget.config changes, test whether cached upstream works

* Update NuGet.Config

* Update NuGet.Config

---------

Co-authored-by: Yash Patil <[email protected]>
Co-authored-by: Xiaogang <[email protected]>
Co-authored-by: Yabo Hu <[email protected]>
VeryEarly added a commit that referenced this pull request Aug 19, 2024
* Fixed psObject List Enumeration

* Added test case: UpdateConfig cmdlet

* Updated Changelog

* Additioanl Test Case + Updated Changelog

* Added complex test case: Multiple Configs

* Update NuGet.Config

Fix the security pipeline failure.

* revert nuget.config changes, test whether cached upstream works

* Update NuGet.Config

* Update NuGet.Config

---------

Co-authored-by: Yash <[email protected]>
Co-authored-by: Yash Patil <[email protected]>
Co-authored-by: Xiaogang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants