Skip to content

[KeyVault] Added support for ECC to Key Vault#1368

Closed
AutorestCI wants to merge 1 commit intoAzure:masterfrom
AutorestCI:RestAPI-PR1538
Closed

[KeyVault] Added support for ECC to Key Vault#1368
AutorestCI wants to merge 1 commit intoAzure:masterfrom
AutorestCI:RestAPI-PR1538

Conversation

@AutorestCI
Copy link
Copy Markdown
Contributor

Generated from RestAPI PR: Azure/azure-rest-api-specs#1538

@codecov-io
Copy link
Copy Markdown

codecov-io commented Aug 16, 2017

Codecov Report

Merging #1368 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1368      +/-   ##
==========================================
+ Coverage   54.82%   54.82%   +<.01%     
==========================================
  Files        3012     3012              
  Lines       75844    75858      +14     
==========================================
+ Hits        41578    41592      +14     
  Misses      34266    34266
Impacted Files Coverage Δ
...re/keyvault/models/certificate_merge_parameters.py 62.5% <ø> (ø) ⬆️
...ure/keyvault/models/x509_certificate_properties.py 100% <ø> (ø) ⬆️
...dels/pending_certificate_signing_request_result.py 83.33% <ø> (ø) ⬆️
.../keyvault/models/deleted_certificate_item_paged.py 100% <ø> (ø) ⬆️
...e-keyvault/azure/keyvault/models/key_item_paged.py 100% <ø> (ø) ⬆️
...yvault/azure/keyvault/models/certificate_bundle.py 100% <ø> (ø) ⬆️
...lt/azure/keyvault/models/deleted_key_item_paged.py 100% <ø> (ø) ⬆️
...yvault/azure/keyvault/models/deleted_key_bundle.py 100% <ø> (ø) ⬆️
...ault/azure/keyvault/models/storage_account_item.py 55.55% <ø> (ø) ⬆️
...lt/azure/keyvault/models/certificate_item_paged.py 100% <ø> (ø) ⬆️
... and 73 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 631a6c3...9f7326d. Read the comment docs.

@lmazuel lmazuel requested a review from schaabs August 17, 2017 15:48
@lmazuel lmazuel changed the title Automatic PR from RestAPI-PR1538 [KeyVault] Added support for ECC to Key Vault Aug 17, 2017
Copy link
Copy Markdown
Member

@lmazuel lmazuel left a comment

Choose a reason for hiding this comment

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

"curve" seems to have been added in the middle of the parameters list. If it's not on purpose, it's unfortunate because it's a breaking change that could be avoided by just putting new optional parameter at the end of the Swagger


def create_key(
self, vault_base_url, key_name, kty, key_size=None, key_ops=None, key_attributes=None, tags=None, custom_headers=None, raw=False, **operation_config):
self, vault_base_url, key_name, kty, curve=None, key_size=None, key_ops=None, key_attributes=None, tags=None, custom_headers=None, raw=False, **operation_config):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is breaking, and might be avoided by just putting this parameter later at the end in the parameter list in the Swagger.

:rtype: :class:`KeyBundle <azure.keyvault.models.KeyBundle>`
:rtype: :class:`ClientRawResponse<msrest.pipeline.ClientRawResponse>`
if raw=true
:return: :class:`KeyBundle <azure.keyvault.models.KeyBundle>` or
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

FYI all these docstring update are new Autorest, improved by the feedback from docs.microsoft.com

path_format_arguments = {
'vaultBaseUrl': self._serialize.url("vault_base_url", vault_base_url, 'str', skip_quote=True),
'key-name': self._serialize.url("key_name", key_name, 'str', pattern='^[0-9a-zA-Z-]+$')
'key-name': self._serialize.url("key_name", key_name, 'str', pattern=r'^[0-9a-zA-Z-]+$')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Autorest update, that's normal

class CertificateIssuerItemPaged(Paged):
"""
A paging container for iterating over a list of CertificateIssuerItem object
A paging container for iterating over a list of :class:`CertificateIssuerItem <azure.keyvault.models.CertificateIssuerItem>` object
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Autorest update, that's ok

Copy link
Copy Markdown

@schaabs schaabs left a comment

Choose a reason for hiding this comment

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

Rejecting this change. We're going to update the swagger so the generated code is backwards compatible.

@schaabs schaabs closed this Aug 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants