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.

Sign up for GitHub

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

Jump to bottom

Resolve architects' review feedback for Key Vault #15580

Closed
1 task
heaths opened this issue Jun 5, 2021 · 0 comments · Fixed by #15586
Closed
1 task

Resolve architects' review feedback for Key Vault #15580

heaths opened this issue Jun 5, 2021 · 0 comments · Fixed by #15586
Assignees
@maorleger
Labels
Client This issue points to a problem in the data-plane of the library. KeyVault
Milestone
[2021] June

Comments

@heaths
Copy link
Member

heaths commented Jun 5, 2021

Feedback from our architects:

  • Delete methods on the KeyVaultAccessControlClient should return void and ignore HTTP 404s.

Feel free to update this issue with idiomatic feedback.
@heaths heaths added Client This issue points to a problem in the data-plane of the library. KeyVault labels Jun 5, 2021
@heaths heaths added this to the [2021] June milestone Jun 5, 2021
@maorleger maorleger mentioned this issue Jun 7, 2021
Merged
@heaths heaths changed the title Resole architects' review feedback for Key Vault Resolve architects' review feedback for Key Vault Jun 7, 2021
maorleger added a commit that referenced this issue Jun 8, 2021
@maorleger
[KeyVault] - Delete methods should ignore 404 (#15586)
2089881 
## What

- Ensure KeyVaultAccessControlClient delete* methods ignore 404s and return the raw response instead of the deleted model.
- Update tests to use `supportsTracing` helper

## Why

- This came up in a recent architecture discussion. By succeeding when trying to delete a role definition that was already deleted we make retries much easier for the consumer. 
- The `supportsTracing` helper was added after these tests were written and is much more comprehensive.

Resolves #15580
openapi-sdkautomation bot pushed a commit to AzureSDKAutomation/azure-sdk-for-js that referenced this issue Aug 11, 2021
@cschen13
CodeGen from PR 15548 in Azure/azure-rest-api-specs
8dad3b7 
Web ant94 2021 02 01 attempt 2 (Azure#15548)

* Adds base for updating Microsoft.Web from version stable/2021-01-15 to version 2021-02-01

* Updates readme

* Updates API version in new specs and examples

* Changes for api-version 2012-02-01 (Azure#15545)

* Address PR comments - AppServiceCertificateOrders (Azure#15561)

* Changes for api-version 2012-02-01

* Address PR comments - AppServiceCertificatesOrder

* Fix spelling issue

* Fix summary+description for GET siteseal

* Add reseller to custom words

* Make DedicatedHostCount R/W (Azure#15580)

Co-authored-by: Chris Chen <[email protected]>
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@maorleger maorleger

Labels
Client This issue points to a problem in the data-plane of the library. KeyVault
Projects
None yet
Milestone
[2021] June
Development

Successfully merging a pull request may close this issue.

[KeyVault] - Delete methods should ignore 404 maorleger/azure-sdk-for-js
2 participants
@maorleger @heaths