Skip to content

Conversation

@mccoyp
Copy link
Member

@mccoyp mccoyp commented Nov 5, 2020

Part of fix to #13006.

Merged implementation for certificates: #14518.

@mccoyp mccoyp added KeyVault Client This issue points to a problem in the data-plane of the library. labels Nov 5, 2020
@mccoyp mccoyp added this to the [2020] November milestone Nov 5, 2020
iscai-msft
iscai-msft previously approved these changes Nov 6, 2020
**kwargs
)
return UnwrapResult(key_id=self._key_id, algorithm=algorithm, key=operation_result.result)
return UnwrapResult(key_id=self._key_id.source_id, algorithm=algorithm, key=operation_result.result)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return UnwrapResult(key_id=self._key_id.source_id, algorithm=algorithm, key=operation_result.result)
return UnwrapResult(key_id=self.key_id, algorithm=algorithm, key=operation_result.result)

print(parsed_key_id.version)
print(parsed_key_id.source_id)
# [END parse_key_vault_key_id]
self.assertEqual(parsed_key_id.name, key_name)
Copy link
Member

Choose a reason for hiding this comment

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

New tests should use assert instead of unittest.TestCase assertions:

Suggested change
self.assertEqual(parsed_key_id.name, key_name)
assert parsed_key_id.name == key_name

from _shared.test_case import KeyVaultTestCase

# pre-apply the client_cls positional argument so it needn't be explicitly passed below
KeyVaultClientPreparer = functools.partial(_KeyVaultClientPreparer, KeyClient)
Copy link
Member

Choose a reason for hiding this comment

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

Nitpicking, but this isn't really an improvement because this module only uses the preparer once.

@mccoyp mccoyp merged commit 599bde3 into Azure:master Nov 7, 2020
@mccoyp mccoyp deleted the parseid branch November 7, 2020 00:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Client This issue points to a problem in the data-plane of the library. KeyVault

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants