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.

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

Run mypy in azure-keyvault-secrets CI #20507

Merged
merged 16 commits into from
Sep 8, 2021
1 change: 1 addition & 0 deletions eng/tox/mypy_hard_failure_packages.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
"azure-identity",
"azure-keyvault-administration",
"azure-keyvault-certificates",
"azure-keyvault-secrets",
"azure-servicebus",
"azure-ai-textanalytics",
"azure-ai-formrecognizer",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ class KeyVaultOperationPoller(LROPoller):
# pylint: disable=arguments-differ
def __init__(self, polling_method):
# type: (PollingMethod) -> None
super(KeyVaultOperationPoller, self).__init__(None, None, None, NoPolling())
super(KeyVaultOperationPoller, self).__init__(None, None, lambda *_: None, NoPolling())
self._polling_method = polling_method

# pylint: disable=arguments-differ
def result(self):
def result(self): # type: ignore
# type: () -> Any
"""Returns a representation of the final resource without waiting for the operation to complete.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
# ------------------------------------
from functools import partial
from azure.core.tracing.decorator import distributed_trace
from azure.core.polling import LROPoller

from ._models import KeyVaultSecret, DeletedSecret, SecretProperties
from ._shared import KeyVaultClientBase
Expand Down Expand Up @@ -294,7 +295,7 @@ def restore_secret_backup(self, backup, **kwargs):

@distributed_trace
def begin_delete_secret(self, name, **kwargs):
# type: (str, **Any) -> DeletedSecret
# type: (str, **Any) -> LROPoller
"""Delete all versions of a secret. Requires secrets/delete permission.

When this method returns Key Vault has begun deleting the secret. Deletion may take several seconds in a vault
Expand Down Expand Up @@ -416,7 +417,7 @@ def purge_deleted_secret(self, name, **kwargs):

@distributed_trace
def begin_recover_deleted_secret(self, name, **kwargs):
# type: (str, **Any) -> SecretProperties
# type: (str, **Any) -> LROPoller
"""Recover a deleted secret to its latest version. Possible only in a vault with soft-delete enabled.

If the vault does not have soft-delete enabled, :func:`begin_delete_secret` is permanent, and this method will
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ class SecretProperties(object):
"""A secret's id and attributes."""

def __init__(self, attributes, vault_id, **kwargs):
# type: (_models.SecretAttributes, str, **Any) -> None
# type: (Optional[_models.SecretAttributes], Optional[str], **Any) -> None
self._attributes = attributes
self._id = vault_id
self._vault_id = KeyVaultSecretIdentifier(vault_id)
self._vault_id = KeyVaultSecretIdentifier(vault_id) if vault_id else None
YalinLi0312 marked this conversation as resolved.
Show resolved Hide resolved
self._content_type = kwargs.get("content_type", None)
self._key_id = kwargs.get("key_id", None)
self._managed = kwargs.get("managed", None)
Expand Down Expand Up @@ -60,82 +60,82 @@ def _from_secret_item(cls, secret_item):

@property
def content_type(self):
# type: () -> str
# type: () -> Optional[str]
"""An arbitrary string indicating the type of the secret

:rtype: str
:rtype: str or None
"""
return self._content_type

@property
def id(self):
# type: () -> str
# type: () -> Optional[str]
YalinLi0312 marked this conversation as resolved.
Show resolved Hide resolved
"""The secret's id

:rtype: str
:rtype: str or None
"""
return self._id

@property
def key_id(self):
# type: () -> str
# type: () -> Optional[str]
"""If this secret backs a certificate, this property is the identifier of the corresponding key.

:rtype: str
:rtype: str or None
"""
return self._key_id

@property
def enabled(self):
# type: () -> bool
# type: () -> Optional[bool]
"""Whether the secret is enabled for use

:rtype: bool
:rtype: bool or None
"""
return self._attributes.enabled
return self._attributes.enabled if self._attributes else None

@property
def not_before(self):
# type: () -> datetime
# type: () -> Optional[datetime]
"""The time before which the secret can not be used, in UTC

:rtype: ~datetime.datetime
:rtype: ~datetime.datetime or None
"""
return self._attributes.not_before
return self._attributes.not_before if self._attributes else None

@property
def expires_on(self):
# type: () -> datetime
# type: () -> Optional[datetime]
"""When the secret expires, in UTC

:rtype: ~datetime.datetime
:rtype: ~datetime.datetime or None
"""
return self._attributes.expires
return self._attributes.expires if self._attributes else None

@property
def created_on(self):
# type: () -> datetime
# type: () -> Optional[datetime]
"""When the secret was created, in UTC

:rtype: ~datetime.datetime
:rtype: ~datetime.datetime or None
"""
return self._attributes.created
return self._attributes.created if self._attributes else None

@property
def updated_on(self):
# type: () -> datetime
# type: () -> Optional[datetime]
"""When the secret was last updated, in UTC

:rtype: ~datetime.datetime
:rtype: ~datetime.datetime or None
"""
return self._attributes.updated
return self._attributes.updated if self._attributes else None

@property
def recoverable_days(self):
# type: () -> Optional[int]
"""The number of days the key is retained before being deleted from a soft-delete enabled Key Vault.

:rtype: int
:rtype: int or None
"""
# recoverable_days was added in 7.1-preview
if self._attributes and hasattr(self._attributes, "recoverable_days"):
Expand All @@ -144,54 +144,55 @@ def recoverable_days(self):

@property
def recovery_level(self):
# type: () -> str
# type: () -> Optional[str]
"""The vault's deletion recovery level for secrets

:rtype: str
:rtype: str or None
"""
return self._attributes.recovery_level
return self._attributes.recovery_level if self._attributes else None

@property
def vault_url(self):
# type: () -> str
# type: () -> Optional[str]
"""URL of the vault containing the secret

:rtype: str
:rtype: str or None
"""
return self._vault_id.vault_url
return self._vault_id.vault_url if self._vault_id else None

@property
def name(self):
# type: () -> str
# type: () -> Optional[str]
"""The secret's name

:rtype: str
:rtype: str or None
"""
return self._vault_id.name
return self._vault_id.name if self._vault_id else None

@property
def version(self):
# type: () -> str
# type: () -> Optional[str]
"""The secret's version

:rtype: str
:rtype: str or None
"""
return self._vault_id.version
return self._vault_id.version if self._vault_id else None

@property
def tags(self):
# type: () -> Dict[str, str]
# type: () -> Optional[Dict[str, str]]
"""Application specific metadata in the form of key-value pairs

:rtype: dict"""
:rtype: dict or None
"""
return self._tags


class KeyVaultSecret(object):
"""All of a secret's properties, and its value."""

def __init__(self, properties, value):
# type: (SecretProperties, str) -> None
# type: (SecretProperties, Optional[str]) -> None
Copy link
Member

Choose a reason for hiding this comment

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

This is a case where the type checking gets a little confusing. A SecretBundle can have a None value, which is why this Optional[str] type makes sense -- but also, SecretSetParameters requires a non-None value. We have to send over SecretSetParameters to create a secret in the first place, meaning that -- in theory -- any secret we get back and turn into a KeyVaultSecret should have a value for value. I don't know what to do about this yet, but I'm thinking aloud a bit about why this is awkward.

Copy link
Member

Choose a reason for hiding this comment

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

When does SecretBundle have a None value?

Copy link
Member

Choose a reason for hiding this comment

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

It doesn't in any real scenario I've seen (and I doubt it ever would), but its type signature leaves the possibility open.

Copy link
Member Author

Choose a reason for hiding this comment

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

Then this introduces one question that happened several times in mypy ci:
when swagger defines a property to be optional while we think there's no chance to be None, should we ignore mypy error in this case?

self._properties = properties
self._value = value

Expand All @@ -210,19 +211,19 @@ def _from_secret_bundle(cls, secret_bundle):

@property
def name(self):
# type: () -> str
# type: () -> Optional[str]
"""The secret's name

:rtype: str
:rtype: str or None
"""
return self._properties.name

@property
def id(self):
# type: () -> str
# type: () -> Optional[str]
"""The secret's id

:rtype: str
:rtype: str or None
"""
return self._properties.id

Expand All @@ -237,10 +238,10 @@ def properties(self):

@property
def value(self):
# type: () -> str
# type: () -> Optional[str]
"""The secret's value

:rtype: str
:rtype: str or None
"""
return self._value

Expand Down Expand Up @@ -329,19 +330,19 @@ def _from_deleted_secret_item(cls, deleted_secret_item):

@property
def name(self):
# type: () -> str
# type: () -> Optional[str]
"""The secret's name

:rtype: str
:rtype: str or None
"""
return self._properties.name

@property
def id(self):
# type: () -> str
# type: () -> Optional[str]
"""The secret's id

:rtype: str
:rtype: str or None
"""
return self._properties.id

Expand All @@ -356,27 +357,27 @@ def properties(self):

@property
def deleted_date(self):
# type: () -> datetime
# type: () -> Optional[datetime]
"""When the secret was deleted, in UTC

:rtype: ~datetime.datetime
:rtype: ~datetime.datetime or None
"""
return self._deleted_date

@property
def recovery_id(self):
# type: () -> str
# type: () -> Optional[str]
"""An identifier used to recover the deleted secret. Returns ``None`` if soft-delete is disabled.

:rtype: str
:rtype: str or None
"""
return self._recovery_id

@property
def scheduled_purge_date(self):
# type: () -> datetime
# type: () -> Optional[datetime]
"""When the secret is scheduled to be purged, in UTC. Returns ``None`` if soft-delete is disabled.

:rtype: ~datetime.datetime
:rtype: ~datetime.datetime or None
"""
return self._scheduled_purge_date
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ class KeyVaultOperationPoller(LROPoller):
# pylint: disable=arguments-differ
def __init__(self, polling_method):
# type: (PollingMethod) -> None
super(KeyVaultOperationPoller, self).__init__(None, None, None, NoPolling())
super(KeyVaultOperationPoller, self).__init__(None, None, lambda *_: None, NoPolling())
self._polling_method = polling_method

# pylint: disable=arguments-differ
def result(self):
def result(self): # type: ignore
# type: () -> Any
"""Returns a representation of the final resource without waiting for the operation to complete.

Expand Down
7 changes: 7 additions & 0 deletions sdk/keyvault/azure-keyvault-secrets/mypy.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[mypy]
python_version = 3.6
warn_unused_configs = True
ignore_missing_imports = True

[mypy-azure.keyvault.*._generated.*]
ignore_errors = True