[KeyVault] Add --no-wait for command az keyvault security-domain download and --target-operation for command az keyvault security-domain wait
#17263
Conversation
| status = 'InProgress' | ||
|
|
||
| # keep polling if status is 'InProgress' | ||
| while status == 'InProgress': |
There was a problem hiding this comment.
set a timeout for Inprogress polling?
There was a problem hiding this comment.
does that mean CLI user will wait till operation finished. For aync operation, shouldn't the polling be done in --wait ?
There was a problem hiding this comment.
That's right. Implementation is updated now.
There was a problem hiding this comment.
set a timeout for Inprogress polling?
I reused the existing codes which supports max_retry now, although i think we should not set it
houk-ms
changed the title
--no-wait for command az keyvault security-domain download and --target-operation for command az keyvault security-domain wait
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
| c.argument('identifier', options_list=['--id'], validator=validate_vault_or_hsm, help='Id of the HSM.') | ||
| c.argument('resource_group_name', options_list=['--resource-group', '-g'], | ||
| help='Proceed only if HSM belongs to the specified resource group.') | ||
| c.argument('target_operation', arg_type=get_enum_type(CLISecurityDomainOperation), |
There was a problem hiding this comment.
final command will be ?
az keyvault security-domain wait --target_operation upload
There was a problem hiding this comment.
--target_operation defaults to upload in avoid of breaking change. so either specifying --target-operation or not would work for upload.
for download, we have to specify --target-operation download
| try: | ||
| with open(file_path, 'w') as f: | ||
| f.write(security_domain.value) | ||
| except: # pylint: disable=bare-except |
There was a problem hiding this comment.
any error to end user when save failed?
There was a problem hiding this comment.
These are the original implementations.
But you are right, we can show some error message here.
| if not no_wait: | ||
| polling_ret = _wait_security_domain_operation(client, hsm_name, 'download') | ||
| # Due to service defect, status could be 'Success' or 'Succeeded' when it succeeded | ||
| if polling_ret and getattr(polling_ret, 'status', None) != 'Failed': |
There was a problem hiding this comment.
pls lower/upper case then compare on string
There was a problem hiding this comment.
I prefer we rely on service in avoid of risks in possible type convertion.
|
|
||
| # v7.2-preview and v7.2 will change the upload operation from Sync to Async | ||
| # due to service defects, it returns 'Succeeded' before the change and 'Success' after the change | ||
| if ret and getattr(ret, 'status', None) in ['Succeeded', 'Success', 'Failed']: |
There was a problem hiding this comment.
align on case before string comparasion, to avoid service change
| query_parameters = {} | ||
| query_parameters['api-version'] = self._serialize.query("self.api_version", self.api_version, 'str') | ||
|
|
||
| # Construct headers |
|
Service team confirmed the changes work as expected. |
2 participants
Description
The security-domain download operation will introduce a breaking change from sync to async. The corresponding changs in swagger are as below
This PR works to support both sync and async operation before the changes are deployed in service side, so that we won't break the users in the latest CLI version. While the old versions of CLI will be broken anyway once the service changes are deployed, of which service team is clearly aware and would like us to do so.
The changes in the PR include
--no-waitparamter for commandaz keyvault security-domain download--target-operationparameter foraz keyvault security-domain wait, so that users could choose to wait eitheruploadordownloadoperation to finish. (Previously, the command is only foruploadoperation, so the paramter defaults touploadhere in avoid of breaking change)--idto specify URL of HSM for commandaz keyvault security-domain upload/download/wait, as discussed in az keyvault security-domain download does not support --id parameter for Managed HSM #17040Testing 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 feature.
This 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.