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

More useful exceptions for Key Vault errors #7086

Merged
merged 7 commits into from
Sep 18, 2019
Merged

More useful exceptions for Key Vault errors #7086

merged 7 commits into from
Sep 18, 2019

Conversation

chlowell
Copy link
Member

@chlowell chlowell commented Sep 6, 2019 •
edited
Loading

This ensures exceptions raised in lieu of the autorest-generated KeyVaultErrorException surface Key Vault's error text.

For example, given a 404 response, ResourceNotFoundError is more to the point than KeyVaultErrorException. We can use the error_map kwarg to raise ResourceNotFoundError instead, but without some extra code as in this PR, Key Vault's error text isn't surfaced:

  • generated exception: KeyVaultErrorException: (KeyNotFound) Key not found: imaginary-key
  • naively using error_map: ResourceNotFoundError: Operation returned an invalid status 'Not Found'
  • with this change: ResourceNotFoundError: (KeyNotFound) Key not found: imaginary-key

Closes #7003

@chlowell chlowell added KeyVault Client This issue points to a problem in the data-plane of the library. labels Sep 6, 2019
@chlowell chlowell requested a review from schaabs as a code owner September 6, 2019 00:13
@chlowell chlowell self-assigned this Sep 6, 2019
@adxsdk6
Copy link

adxsdk6 commented Sep 6, 2019

Can one of the admins verify this patch?

johanste
johanste reviewed Sep 6, 2019
View reviewed changes
sdk/keyvault/azure-keyvault-keys/azure/keyvault/keys/_shared/exceptions.py Outdated
_error_map = {404: ResourceNotFoundError, 409: ResourceExistsError}


def raise_from_key_vault_exception(exception):
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious as to why we couldn't use the error_map parameter instead of creating yet another way of mapping errors from autorest generated code?

Copy link
Member Author

Choose a reason for hiding this comment

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

We could, I've reworked this to do that.

@chlowell chlowell requested a review from johanste September 11, 2019 21:32
@chlowell chlowell mentioned this pull request Sep 12, 2019
Merged
@chlowell chlowell requested a review from bryevdv September 17, 2019 18:18
@chlowell chlowell merged commit ee3ad2a into Azure:master Sep 18, 2019
@chlowell chlowell deleted the exceptions branch September 18, 2019 14:00
rakshith91 pushed a commit that referenced this pull request Sep 19, 2019
Fix to have no content type when request body is empty (#7343)
082fd16 
* Correct return types for list_secrets, _versions (#7268)

* More useful exceptions for Key Vault errors (#7086)

* Fix to have no content type when request body is empty
rakshith91 pushed a commit that referenced this pull request Oct 1, 2019
@rakshith91
Fix to have no content type when request body is empty (#7343)
ad3edc9 
* Correct return types for list_secrets, _versions (#7268)

* More useful exceptions for Key Vault errors (#7086)

* Fix to have no content type when request body is empty
@chlowell chlowell mentioned this pull request Oct 4, 2019
Merged
zezha-msft pushed a commit that referenced this pull request Oct 8, 2019
@zezha-msft
Storage Preview 4 release (#7527)
e2266eb 
* Update msrest and regenerate swagger. (#7308)

* Update msrest and regenerate swagger.

* override to get green CI.

* Fix to have no content type when request body is empty (#7343)

* Correct return types for list_secrets, _versions (#7268)

* More useful exceptions for Key Vault errors (#7086)

* Fix to have no content type when request body is empty

* [Blob] Set tier and rehydrate (#7269)

* [BlockBlob][Tier]Add BlobTier Support For CopyBlob/PutBlob/PutBlockList

* [BlockBlob][Tier]Add Recordings for Standard BlobTier Support

* [BlockBlob][Rehydrate]Enable Rehydrate Priority for Copy/SetTier

* [BlockBlob][Rehydrate]Recordings for Rehydrate Priority for Copy/SetTier

* [Blob] Echo client (#7350)

* [EchoClientId]Verify Client Request ID in Response Same as in Request

* [BlockBlob][Rehydrate]Fix Recordings

* Refactor max_connections to max_concurrency (#7531)

* Rename max_connections to max_concurrency

* Other non breaking changes

* comment changes

* Use generated version as constant API version (#7547)

* Re-recording from #7555 (#7585)

* Storage batch API (#7387)

* delete_blobs POC

* Storage POC WIP

* Storage POC async

* Storage WIP

* Storage clean-up

* Storage options

* Autorest remark

* Use config option

* SetBlobTier WIP

* Working and tested delete_blobs + options

* Set standard tier with tests

* Adapt tests for playback mode

* Move batch_send to common mixin

* Python 2.7 compat

* Fix snapshot bug

* Re-record

* Pass kwargs to batch

* Clean x-ms-client-request-id from body if multipart response

* remove duplicated comp=tier

* Disable batch tests on Python 2.7

* pylint

* Batch ChangeLog (#7593)

* [Storage Blob] Regenerate Swagger (#7572)

* Regenerate Swagger

* small fix

* adjusts literalincludes for msft docs (#7599)

* Download sparse blob (#7555)

* [PageBlob]Optimize Download Sparse File

* Breaking Changes - Permission models (#7517)

* Rename Permission related models to SasPermissions

* Rewrite Permission Models

* refactor tests

* some tests

* comment changes

* remove length checks

* QueuePermissions

* File Permissions

* blobs minor tweaks

* minor fix

* pylint fix

* pylint

* Fix batch docstrings (#7600)

* Fix batch docstrings

* pylint

* [Rename]rename max_connections to max_concurrency (#7606)

* small edits to lease client docs (#7550)

* small edits to lease client docs

* whitespace

* Setup identity for unified pipelines. (#7578)

* Setup keyvault for unified pipelines. (#7579)

* Setup eventhubs for publishing via unified pipelines. (#7580)

* Setup eventhubs for publishing via unified pipelines.

* Fixed missing punctuation.

* Setup cosmos for release via unified pipelines. (#7581)

* bump CI

* Manual network september (#7576)

* generated files

* adding 2019-07-01

* history and version

* rerun tests

* updated version

* removed subscription id from tests

* rerun tests again

* replaced subscription id

* [AutoPR] reservations/resource-manager (#7569)

* [AutoPR reservations/resource-manager] [Hub Generated] Review request for Microsoft.Capacity to add version preview/2019-04-01 (#7251)

* Generated from f19a2e5b7f384018b74b21b7b8b8782d95b456f9

fixed x-ms-enum value

* Generated from 2f56008117d578bec6cc8b8c832926f45a0fe52e

fixed catalog definition

* Generated from cac978330e8c7b9583812a735cfeac97fb267056

reverted breaking operation id change

* added version and history

* [AutoPR] sql/resource-manager (#7535)

* Generated from 9402dbf3fb3d5fffcb80f501b1fc857ff97ab723 (#7329)

Fixing PR validation errors

* regenerated

* history and version

* Setup app configuration with unified pipelines. (#7582)

* [AutoPR] resources/resource-manager (#7549)

* regenerated

* updated history and version

* updated date

* Correct import_key parameter type (#7590)

* add doc for tracing (#7616)

* Add a from_blob_url method (#7567)

* Add a from_blob_url method

* Update tests

* Recordings - common blob

* Revert "Recordings - common blob"

This reverts commit ad07f34c25f7662cbaed8b33ac05a1a6f6270dae.

* comment changes

* undo get_client changes

* redo recordings

* from_container_url

* fix tests

* tests fix for container url

* pylint fix

* Fix some docstring

* docstrings

* Doc imprvment for Storage (#7601)

* Docstring improvement

* File docstring

* Queue docstrings

* More doc fixes

* More doc fixes

* Revert "small edits to lease client docs (#7550)" (#7631)

This reverts commit ed20b58.

* kwarg-ify methods (#7611)

* changeset1

* changeset-2

* changeset-3

* changeset-4

* minor fix

* some final tweaks

* pylint

* minor fixes

* max_concurrency

* lease_id

* pylint

* fix

* [Storage] Consolidate offset and range parameters (#7598)

* Updated blob sync tests

* Offset refactor APIS (#19)

* rename to offset and length

* fix

* more changes

* Updated blob sync tests

* Temp block async tests

* Fix upload page behaviour

* Fix clear page behaviour

* Update from_url offset behaviour

* Update page ranges

* Fix download blob behaviour

* Some cleanup

* Fixed page size

* More test fixes

* Some more fixes

* Fixed page tests

* Fixed encryption tests

* Fix common test

* append anf page blob async

* some more test fixes

* Fix live tests

* more changes async

* Fix sparse blob test

* Last tests

* update docstrings

* pylint

* Some Final tweaks (#7653)

* Some final tweaks

* comments

* Fix live tests (#7665)

* fix live tests

* oops

* few changes
YijunXieMS pushed a commit to YijunXieMS/azure-sdk-for-python that referenced this pull request Oct 9, 2019
@chlowell
More useful exceptions for Key Vault errors (Azure#7086)
1a369c3
YijunXieMS pushed a commit to YijunXieMS/azure-sdk-for-python that referenced this pull request Oct 9, 2019
Storage Preview 4 release (Azure#7527)
2a501c9 
* Update msrest and regenerate swagger. (Azure#7308)

* Update msrest and regenerate swagger.

* override to get green CI.

* Fix to have no content type when request body is empty (Azure#7343)

* Correct return types for list_secrets, _versions (Azure#7268)

* More useful exceptions for Key Vault errors (Azure#7086)

* Fix to have no content type when request body is empty

* [Blob] Set tier and rehydrate (Azure#7269)

* [BlockBlob][Tier]Add BlobTier Support For CopyBlob/PutBlob/PutBlockList

* [BlockBlob][Tier]Add Recordings for Standard BlobTier Support

* [BlockBlob][Rehydrate]Enable Rehydrate Priority for Copy/SetTier

* [BlockBlob][Rehydrate]Recordings for Rehydrate Priority for Copy/SetTier

* [Blob] Echo client (Azure#7350)

* [EchoClientId]Verify Client Request ID in Response Same as in Request

* [BlockBlob][Rehydrate]Fix Recordings

* Refactor max_connections to max_concurrency (Azure#7531)

* Rename max_connections to max_concurrency

* Other non breaking changes

* comment changes

* Use generated version as constant API version (Azure#7547)

* Re-recording from Azure#7555 (Azure#7585)

* Storage batch API (Azure#7387)

* delete_blobs POC

* Storage POC WIP

* Storage POC async

* Storage WIP

* Storage clean-up

* Storage options

* Autorest remark

* Use config option

* SetBlobTier WIP

* Working and tested delete_blobs + options

* Set standard tier with tests

* Adapt tests for playback mode

* Move batch_send to common mixin

* Python 2.7 compat

* Fix snapshot bug

* Re-record

* Pass kwargs to batch

* Clean x-ms-client-request-id from body if multipart response

* remove duplicated comp=tier

* Disable batch tests on Python 2.7

* pylint

* Batch ChangeLog (Azure#7593)

* [Storage Blob] Regenerate Swagger (Azure#7572)

* Regenerate Swagger

* small fix

* adjusts literalincludes for msft docs (Azure#7599)

* Download sparse blob (Azure#7555)

* [PageBlob]Optimize Download Sparse File

* Breaking Changes - Permission models (Azure#7517)

* Rename Permission related models to SasPermissions

* Rewrite Permission Models

* refactor tests

* some tests

* comment changes

* remove length checks

* QueuePermissions

* File Permissions

* blobs minor tweaks

* minor fix

* pylint fix

* pylint

* Fix batch docstrings (Azure#7600)

* Fix batch docstrings

* pylint

* [Rename]rename max_connections to max_concurrency (Azure#7606)

* small edits to lease client docs (Azure#7550)

* small edits to lease client docs

* whitespace

* Setup identity for unified pipelines. (Azure#7578)

* Setup keyvault for unified pipelines. (Azure#7579)

* Setup eventhubs for publishing via unified pipelines. (Azure#7580)

* Setup eventhubs for publishing via unified pipelines.

* Fixed missing punctuation.

* Setup cosmos for release via unified pipelines. (Azure#7581)

* bump CI

* Manual network september (Azure#7576)

* generated files

* adding 2019-07-01

* history and version

* rerun tests

* updated version

* removed subscription id from tests

* rerun tests again

* replaced subscription id

* [AutoPR] reservations/resource-manager (Azure#7569)

* [AutoPR reservations/resource-manager] [Hub Generated] Review request for Microsoft.Capacity to add version preview/2019-04-01 (Azure#7251)

* Generated from f19a2e5b7f384018b74b21b7b8b8782d95b456f9

fixed x-ms-enum value

* Generated from 2f56008117d578bec6cc8b8c832926f45a0fe52e

fixed catalog definition

* Generated from cac978330e8c7b9583812a735cfeac97fb267056

reverted breaking operation id change

* added version and history

* [AutoPR] sql/resource-manager (Azure#7535)

* Generated from 9402dbf3fb3d5fffcb80f501b1fc857ff97ab723 (Azure#7329)

Fixing PR validation errors

* regenerated

* history and version

* Setup app configuration with unified pipelines. (Azure#7582)

* [AutoPR] resources/resource-manager (Azure#7549)

* regenerated

* updated history and version

* updated date

* Correct import_key parameter type (Azure#7590)

* add doc for tracing (Azure#7616)

* Add a from_blob_url method (Azure#7567)

* Add a from_blob_url method

* Update tests

* Recordings - common blob

* Revert "Recordings - common blob"

This reverts commit ad07f34c25f7662cbaed8b33ac05a1a6f6270dae.

* comment changes

* undo get_client changes

* redo recordings

* from_container_url

* fix tests

* tests fix for container url

* pylint fix

* Fix some docstring

* docstrings

* Doc imprvment for Storage (Azure#7601)

* Docstring improvement

* File docstring

* Queue docstrings

* More doc fixes

* More doc fixes

* Revert "small edits to lease client docs (Azure#7550)" (Azure#7631)

This reverts commit ed20b58.

* kwarg-ify methods (Azure#7611)

* changeset1

* changeset-2

* changeset-3

* changeset-4

* minor fix

* some final tweaks

* pylint

* minor fixes

* max_concurrency

* lease_id

* pylint

* fix

* [Storage] Consolidate offset and range parameters (Azure#7598)

* Updated blob sync tests

* Offset refactor APIS (Azure#19)

* rename to offset and length

* fix

* more changes

* Updated blob sync tests

* Temp block async tests

* Fix upload page behaviour

* Fix clear page behaviour

* Update from_url offset behaviour

* Update page ranges

* Fix download blob behaviour

* Some cleanup

* Fixed page size

* More test fixes

* Some more fixes

* Fixed page tests

* Fixed encryption tests

* Fix common test

* append anf page blob async

* some more test fixes

* Fix live tests

* more changes async

* Fix sparse blob test

* Last tests

* update docstrings

* pylint

* Some Final tweaks (Azure#7653)

* Some final tweaks

* comments

* Fix live tests (Azure#7665)

* fix live tests

* oops

* few changes
fengzhou-msft pushed a commit that referenced this pull request Nov 5, 2019
@fengzhou-msft
Storage Preview 4 release (#7527)
1eca87d 
* Update msrest and regenerate swagger. (#7308)

* Update msrest and regenerate swagger.

* override to get green CI.

* Fix to have no content type when request body is empty (#7343)

* Correct return types for list_secrets, _versions (#7268)

* More useful exceptions for Key Vault errors (#7086)

* Fix to have no content type when request body is empty

* [Blob] Set tier and rehydrate (#7269)

* [BlockBlob][Tier]Add BlobTier Support For CopyBlob/PutBlob/PutBlockList

* [BlockBlob][Tier]Add Recordings for Standard BlobTier Support

* [BlockBlob][Rehydrate]Enable Rehydrate Priority for Copy/SetTier

* [BlockBlob][Rehydrate]Recordings for Rehydrate Priority for Copy/SetTier

* [Blob] Echo client (#7350)

* [EchoClientId]Verify Client Request ID in Response Same as in Request

* [BlockBlob][Rehydrate]Fix Recordings

* Refactor max_connections to max_concurrency (#7531)

* Rename max_connections to max_concurrency

* Other non breaking changes

* comment changes

* Use generated version as constant API version (#7547)

* Re-recording from #7555 (#7585)

* Storage batch API (#7387)

* delete_blobs POC

* Storage POC WIP

* Storage POC async

* Storage WIP

* Storage clean-up

* Storage options

* Autorest remark

* Use config option

* SetBlobTier WIP

* Working and tested delete_blobs + options

* Set standard tier with tests

* Adapt tests for playback mode

* Move batch_send to common mixin

* Python 2.7 compat

* Fix snapshot bug

* Re-record

* Pass kwargs to batch

* Clean x-ms-client-request-id from body if multipart response

* remove duplicated comp=tier

* Disable batch tests on Python 2.7

* pylint

* Batch ChangeLog (#7593)

* [Storage Blob] Regenerate Swagger (#7572)

* Regenerate Swagger

* small fix

* adjusts literalincludes for msft docs (#7599)

* Download sparse blob (#7555)

* [PageBlob]Optimize Download Sparse File

* Breaking Changes - Permission models (#7517)

* Rename Permission related models to SasPermissions

* Rewrite Permission Models

* refactor tests

* some tests

* comment changes

* remove length checks

* QueuePermissions

* File Permissions

* blobs minor tweaks

* minor fix

* pylint fix

* pylint

* Fix batch docstrings (#7600)

* Fix batch docstrings

* pylint

* [Rename]rename max_connections to max_concurrency (#7606)

* small edits to lease client docs (#7550)

* small edits to lease client docs

* whitespace

* Setup identity for unified pipelines. (#7578)

* Setup keyvault for unified pipelines. (#7579)

* Setup eventhubs for publishing via unified pipelines. (#7580)

* Setup eventhubs for publishing via unified pipelines.

* Fixed missing punctuation.

* Setup cosmos for release via unified pipelines. (#7581)

* bump CI

* Manual network september (#7576)

* generated files

* adding 2019-07-01

* history and version

* rerun tests

* updated version

* removed subscription id from tests

* rerun tests again

* replaced subscription id

* [AutoPR] reservations/resource-manager (#7569)

* [AutoPR reservations/resource-manager] [Hub Generated] Review request for Microsoft.Capacity to add version preview/2019-04-01 (#7251)

* Generated from f19a2e5b7f384018b74b21b7b8b8782d95b456f9

fixed x-ms-enum value

* Generated from 2f56008117d578bec6cc8b8c832926f45a0fe52e

fixed catalog definition

* Generated from cac978330e8c7b9583812a735cfeac97fb267056

reverted breaking operation id change

* added version and history

* [AutoPR] sql/resource-manager (#7535)

* Generated from 9402dbf3fb3d5fffcb80f501b1fc857ff97ab723 (#7329)

Fixing PR validation errors

* regenerated

* history and version

* Setup app configuration with unified pipelines. (#7582)

* [AutoPR] resources/resource-manager (#7549)

* regenerated

* updated history and version

* updated date

* Correct import_key parameter type (#7590)

* add doc for tracing (#7616)

* Add a from_blob_url method (#7567)

* Add a from_blob_url method

* Update tests

* Recordings - common blob

* Revert "Recordings - common blob"

This reverts commit ad07f34c25f7662cbaed8b33ac05a1a6f6270dae.

* comment changes

* undo get_client changes

* redo recordings

* from_container_url

* fix tests

* tests fix for container url

* pylint fix

* Fix some docstring

* docstrings

* Doc imprvment for Storage (#7601)

* Docstring improvement

* File docstring

* Queue docstrings

* More doc fixes

* More doc fixes

* Revert "small edits to lease client docs (#7550)" (#7631)

This reverts commit ed20b58.

* kwarg-ify methods (#7611)

* changeset1

* changeset-2

* changeset-3

* changeset-4

* minor fix

* some final tweaks

* pylint

* minor fixes

* max_concurrency

* lease_id

* pylint

* fix

* [Storage] Consolidate offset and range parameters (#7598)

* Updated blob sync tests

* Offset refactor APIS (#19)

* rename to offset and length

* fix

* more changes

* Updated blob sync tests

* Temp block async tests

* Fix upload page behaviour

* Fix clear page behaviour

* Update from_url offset behaviour

* Update page ranges

* Fix download blob behaviour

* Some cleanup

* Fixed page size

* More test fixes

* Some more fixes

* Fixed page tests

* Fixed encryption tests

* Fix common test

* append anf page blob async

* some more test fixes

* Fix live tests

* more changes async

* Fix sparse blob test

* Last tests

* update docstrings

* pylint

* Some Final tweaks (#7653)

* Some final tweaks

* comments

* Fix live tests (#7665)

* fix live tests

* oops

* few changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bryevdv bryevdv bryevdv approved these changes

@xiangyan99 xiangyan99 Awaiting requested review from xiangyan99

@iscai-msft iscai-msft Awaiting requested review from iscai-msft

@schaabs schaabs Awaiting requested review from schaabs

@johanste johanste Awaiting requested review from johanste

Assignees

@chlowell chlowell

Labels
Client This issue points to a problem in the data-plane of the library. KeyVault
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Clients leak generated KeyVaultErrorException
4 participants
@chlowell @adxsdk6 @bryevdv @johanste