Skip to content

Run mypy in azure-keyvault-secrets CI #20507

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

Merged
YalinLi0312 merged 16 commits into from
Sep 8, 2021
Merged

Run mypy in azure-keyvault-secrets CI #20507

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
e0e2e0c
Fix errors in keyvault secrets
Sep 1, 2021
0863a5d
Fix errors in keyvault certificates shared folder
Sep 1, 2021
622efd0
Fix errors in keyvault keys shared folder
Sep 1, 2021
1c95057
Update mypy hard failures file
Sep 1, 2021
251393a
Update format
Sep 1, 2021
2f028b7
Add mypy.ini to ignore generated folder when running mypy
Sep 2, 2021
fcad93a
Address comments
Sep 2, 2021
e5c5670
Address comments
Sep 2, 2021
2e2629e
Address comments
Sep 2, 2021
91c34d6
WIP
Sep 2, 2021
aff13b0
Address comments
Sep 2, 2021
c7678e3
Address comments
Sep 3, 2021
8d90691
Address comments
Sep 8, 2021
3c99736
Resolve conflicts
Sep 8, 2021
968ab6b
Correct package name in mypy_hard_failure_packages.py
Sep 8, 2021
71c65ed
Address comments
Sep 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions eng/tox/mypy_hard_failure_packages.py
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions sdk/keyvault/azure-keyvault-keys/azure/keyvault/keys/_shared/_polling.py
View file
Open in desktop
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
5 changes: 3 additions & 2 deletions sdk/keyvault/azure-keyvault-secrets/azure/keyvault/secrets/_client.py
View file
Open in desktop
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
111 changes: 56 additions & 55 deletions sdk/keyvault/azure-keyvault-secrets/azure/keyvault/secrets/_models.py
View file
Open in desktop
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
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]
"""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

@mccoyp mccoyp Sep 3, 2021

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

@chlowell chlowell Sep 7, 2021

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

@mccoyp mccoyp Sep 7, 2021

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
Author

@YalinLi0312 YalinLi0312 Sep 8, 2021

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
4 changes: 2 additions & 2 deletions sdk/keyvault/azure-keyvault-secrets/azure/keyvault/secrets/_shared/_polling.py
View file
Open in desktop
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
View file
Open in desktop
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