{Keyvault} Migrate azure-mgmt-keyvault to typespec generated SDK #32419
Conversation
|
Validation for Azure CLI Full Test Starting...
Thanks for your contribution! |
|
Validation for Breaking Change Starting...
Thanks for your contribution! |
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the azure-mgmt-keyvault SDK from version 12.1.0 to 13.0.0, which is a TypeSpec-generated SDK version. The migration includes API compatibility changes and adds output transformers to maintain backward compatibility with CLI users.
Key Changes
- Updated azure-mgmt-keyvault dependency to version 13.0.0 across all platform requirements
- Changed
Permissionsparameter fromkeystokeys_propertythroughout the codebase to match the new SDK API - Added output transformers to convert
keysPropertyback tokeysin CLI output for backward compatibility
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/azure-cli/setup.py | Updated azure-mgmt-keyvault version constraint to 13.0.0 |
| src/azure-cli/requirements.py3.windows.txt | Updated azure-mgmt-keyvault version to 13.0.0 for Windows |
| src/azure-cli/requirements.py3.Linux.txt | Updated azure-mgmt-keyvault version to 13.0.0 for Linux |
| src/azure-cli/requirements.py3.Darwin.txt | Updated azure-mgmt-keyvault version to 13.0.0 for macOS |
| src/azure-cli/azure/cli/command_modules/keyvault/custom.py | Updated API calls to use keys_property parameter and changed get_sdk to get_models; added .result() calls to vault operations (has critical bugs) |
| src/azure-cli/azure/cli/command_modules/keyvault/commands.py | Added transformer functions to vault commands to maintain output backward compatibility |
| src/azure-cli/azure/cli/command_modules/keyvault/_transformers.py | Added new transformer functions to convert keysProperty to keys in output |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| resource_group_name=resource_group_name, | ||
| vault_name=vault_name, | ||
| parameters=parameters) | ||
| parameters=parameters).result() |
There was a problem hiding this comment.
Calling .result() after sdk_no_wait() breaks the no-wait functionality. When no_wait=True, sdk_no_wait returns a poller without waiting, but immediately calling .result() forces the operation to wait anyway, defeating the purpose of the no_wait parameter.
The .result() call should be removed here. The sdk_no_wait function already handles the no_wait parameter correctly by returning either the result (when no_wait=False) or the poller (when no_wait=True).
| location=parameters.location, | ||
| tags=parameters.tags, | ||
| properties=parameters.properties)) | ||
| properties=parameters.properties)).result() |
There was a problem hiding this comment.
Calling .result() after sdk_no_wait() breaks the no-wait functionality. When no_wait=True, sdk_no_wait returns a poller without waiting, but immediately calling .result() forces the operation to wait anyway, defeating the purpose of the no_wait parameter.
The .result() call should be removed here. The sdk_no_wait function already handles the no_wait parameter correctly by returning either the result (when no_wait=False) or the poller (when no_wait=True).
| properties=parameters.properties)).result() | |
| properties=parameters.properties) | |
| ) |
| location=vault.location, | ||
| tags=vault.tags, | ||
| properties=vault.properties)) | ||
| properties=vault.properties)).result() |
There was a problem hiding this comment.
Calling .result() after sdk_no_wait() breaks the no-wait functionality. When no_wait=True, sdk_no_wait returns a poller without waiting, but immediately calling .result() forces the operation to wait anyway, defeating the purpose of the no_wait parameter.
The .result() call should be removed here. The sdk_no_wait function already handles the no_wait parameter correctly by returning either the result (when no_wait=False) or the poller (when no_wait=True).
| properties=vault.properties)).result() | |
| properties=vault.properties)) |
| location=vault.location, | ||
| tags=vault.tags, | ||
| properties=vault.properties)) | ||
| properties=vault.properties)).result() |
There was a problem hiding this comment.
Calling .result() after sdk_no_wait() breaks the no-wait functionality. When no_wait=True, sdk_no_wait returns a poller without waiting, but immediately calling .result() forces the operation to wait anyway, defeating the purpose of the no_wait parameter.
The .result() call should be removed here. The sdk_no_wait function already handles the no_wait parameter correctly by returning either the result (when no_wait=False) or the poller (when no_wait=True).
| properties=vault.properties)).result() | |
| properties=vault.properties)) |
| location=vault.location, | ||
| tags=vault.tags, | ||
| properties=vault.properties)) | ||
| properties=vault.properties)).result() |
There was a problem hiding this comment.
Calling .result() after sdk_no_wait() breaks the no-wait functionality. When no_wait=True, sdk_no_wait returns a poller without waiting, but immediately calling .result() forces the operation to wait anyway, defeating the purpose of the no_wait parameter.
The .result() call should be removed here. The sdk_no_wait function already handles the no_wait parameter correctly by returning either the result (when no_wait=False) or the poller (when no_wait=True).
| properties=vault.properties)).result() | |
| properties=vault.properties)) |
| location=vault.location, | ||
| tags=vault.tags, | ||
| properties=vault.properties)) | ||
| properties=vault.properties)).result() |
There was a problem hiding this comment.
Calling .result() after sdk_no_wait() breaks the no-wait functionality. When no_wait=True, sdk_no_wait returns a poller without waiting, but immediately calling .result() forces the operation to wait anyway, defeating the purpose of the no_wait parameter.
The .result() call should be removed here. The sdk_no_wait function already handles the no_wait parameter correctly by returning either the result (when no_wait=False) or the poller (when no_wait=True).
| properties=vault.properties)).result() | |
| properties=vault.properties)) |
| resource_group_name=resource_group_name, | ||
| vault_name=vault_name, | ||
| parameters=params) | ||
| parameters=params).result() |
There was a problem hiding this comment.
Calling .result() after sdk_no_wait() breaks the no-wait functionality. When no_wait=True, sdk_no_wait returns a poller without waiting, but immediately calling .result() forces the operation to wait anyway, defeating the purpose of the no_wait parameter.
The .result() call should be removed here. The sdk_no_wait function already handles the no_wait parameter correctly by returning either the result (when no_wait=False) or the poller (when no_wait=True).
| parameters=params).result() | |
| parameters=params) |
2 participants
Related command
Description
Testing Guide
History Notes
[Component Name 1] BREAKING CHANGE:
az command a: Make some customer-facing breaking change[Component Name 2]
az command b: Add some customer-facing featureThis checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.