From a99bf88417d6aec03923447c70c2752f6bb5c459 Mon Sep 17 00:00:00 2001 From: Christopher Wilcox Date: Thu, 14 Oct 2021 14:43:01 -0700 Subject: [PATCH] fix: improve type hints, mypy checks (#448) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * chore: ensure mypy passes * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * chore: move type info to inline, not mypy.ini * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * test: add mypy test scenario * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * chore: simplify type of __version__ * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * chore: expand typing verification to google namespace * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * chore: remove ignores on api_core module * chore: no pytype * chore: scope to just google.cloud.bigtable, defer fixing errors on broader package * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * chore: fix template * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * chore: lint * fix: break circular import Remove '_CLIENT_INFO' from module scope. * fix: unsnarl typing around list of retryable status codes * ci: fix coverage gap induced by typing Co-authored-by: Owl Bot Co-authored-by: Tres Seaver --- google/cloud/__init__.py | 4 +++- google/cloud/bigtable/__init__.py | 7 ++++--- google/cloud/bigtable/backup.py | 4 ++-- google/cloud/bigtable/client.py | 21 ++++++++++++--------- google/cloud/bigtable/instance.py | 2 +- google/cloud/bigtable/policy.py | 4 ++-- google/cloud/bigtable/row.py | 6 +++--- google/cloud/bigtable/row_data.py | 6 +++--- google/cloud/bigtable/row_filters.py | 4 ++-- google/cloud/bigtable/row_set.py | 2 +- google/cloud/bigtable/table.py | 17 ++++++++++------- mypy.ini | 6 ++++++ noxfile.py | 10 ++++++++++ owlbot.py | 24 +++++++++++++++++++++++- tests/unit/test_client.py | 12 ++++-------- 15 files changed, 86 insertions(+), 43 deletions(-) create mode 100644 mypy.ini diff --git a/google/cloud/__init__.py b/google/cloud/__init__.py index 2f4b4738a..ced5017a1 100644 --- a/google/cloud/__init__.py +++ b/google/cloud/__init__.py @@ -1,3 +1,5 @@ +from typing import List + try: import pkg_resources @@ -5,4 +7,4 @@ except ImportError: import pkgutil - __path__ = pkgutil.extend_path(__path__, __name__) + __path__: List[str] = pkgutil.extend_path(__path__, __name__) diff --git a/google/cloud/bigtable/__init__.py b/google/cloud/bigtable/__init__.py index f2c5a24bd..a54096624 100644 --- a/google/cloud/bigtable/__init__.py +++ b/google/cloud/bigtable/__init__.py @@ -15,15 +15,16 @@ """Google Cloud Bigtable API package.""" +from typing import Optional import pkg_resources +from google.cloud.bigtable.client import Client + +__version__: Optional[str] try: __version__ = pkg_resources.get_distribution("google-cloud-bigtable").version except pkg_resources.DistributionNotFound: __version__ = None -from google.cloud.bigtable.client import Client - - __all__ = ["__version__", "Client"] diff --git a/google/cloud/bigtable/backup.py b/google/cloud/bigtable/backup.py index 0991e85f5..c2b5ec9ee 100644 --- a/google/cloud/bigtable/backup.py +++ b/google/cloud/bigtable/backup.py @@ -16,12 +16,12 @@ import re -from google.cloud._helpers import _datetime_to_pb_timestamp +from google.cloud._helpers import _datetime_to_pb_timestamp # type: ignore from google.cloud.bigtable_admin_v2 import BigtableTableAdminClient from google.cloud.bigtable_admin_v2.types import table from google.cloud.bigtable.encryption_info import EncryptionInfo from google.cloud.bigtable.policy import Policy -from google.cloud.exceptions import NotFound +from google.cloud.exceptions import NotFound # type: ignore from google.protobuf import field_mask_pb2 _BACKUP_NAME_RE = re.compile( diff --git a/google/cloud/bigtable/client.py b/google/cloud/bigtable/client.py index 7746ee2ae..c50c20b0f 100644 --- a/google/cloud/bigtable/client.py +++ b/google/cloud/bigtable/client.py @@ -29,11 +29,11 @@ """ import os import warnings -import grpc +import grpc # type: ignore -from google.api_core.gapic_v1 import client_info -import google.auth -from google.auth.credentials import AnonymousCredentials +from google.api_core.gapic_v1 import client_info as client_info_lib +import google.auth # type: ignore +from google.auth.credentials import AnonymousCredentials # type: ignore from google.cloud import bigtable_v2 from google.cloud import bigtable_admin_v2 @@ -45,21 +45,20 @@ BigtableTableAdminGrpcTransport, ) -from google.cloud.bigtable import __version__ +from google.cloud import bigtable from google.cloud.bigtable.instance import Instance from google.cloud.bigtable.cluster import Cluster -from google.cloud.client import ClientWithProject +from google.cloud.client import ClientWithProject # type: ignore from google.cloud.bigtable_admin_v2.types import instance from google.cloud.bigtable.cluster import _CLUSTER_NAME_RE -from google.cloud.environment_vars import BIGTABLE_EMULATOR +from google.cloud.environment_vars import BIGTABLE_EMULATOR # type: ignore INSTANCE_TYPE_PRODUCTION = instance.Instance.Type.PRODUCTION INSTANCE_TYPE_DEVELOPMENT = instance.Instance.Type.DEVELOPMENT INSTANCE_TYPE_UNSPECIFIED = instance.Instance.Type.TYPE_UNSPECIFIED -_CLIENT_INFO = client_info.ClientInfo(client_library_version=__version__) SPANNER_ADMIN_SCOPE = "https://www.googleapis.com/auth/spanner.admin" ADMIN_SCOPE = "https://www.googleapis.com/auth/bigtable.admin" """Scope for interacting with the Cluster Admin and Table Admin APIs.""" @@ -155,11 +154,15 @@ def __init__( credentials=None, read_only=False, admin=False, - client_info=_CLIENT_INFO, + client_info=None, client_options=None, admin_client_options=None, channel=None, ): + if client_info is None: + client_info = client_info_lib.ClientInfo( + client_library_version=bigtable.__version__, + ) if read_only and admin: raise ValueError( "A read-only client cannot also perform" "administrative actions." diff --git a/google/cloud/bigtable/instance.py b/google/cloud/bigtable/instance.py index e6e2ac027..9c22aaa79 100644 --- a/google/cloud/bigtable/instance.py +++ b/google/cloud/bigtable/instance.py @@ -24,7 +24,7 @@ from google.cloud.bigtable_admin_v2.types import instance -from google.iam.v1 import options_pb2 +from google.iam.v1 import options_pb2 # type: ignore from google.api_core.exceptions import NotFound diff --git a/google/cloud/bigtable/policy.py b/google/cloud/bigtable/policy.py index f5558b6f0..8396642fb 100644 --- a/google/cloud/bigtable/policy.py +++ b/google/cloud/bigtable/policy.py @@ -15,8 +15,8 @@ import base64 from google.api_core.iam import Policy as BasePolicy -from google.cloud._helpers import _to_bytes -from google.iam.v1 import policy_pb2 +from google.cloud._helpers import _to_bytes # type: ignore +from google.iam.v1 import policy_pb2 # type: ignore """IAM roles supported by Bigtable Instance resource""" BIGTABLE_ADMIN_ROLE = "roles/bigtable.admin" diff --git a/google/cloud/bigtable/row.py b/google/cloud/bigtable/row.py index 3fdc230f7..9127a1aae 100644 --- a/google/cloud/bigtable/row.py +++ b/google/cloud/bigtable/row.py @@ -17,9 +17,9 @@ import struct -from google.cloud._helpers import _datetime_from_microseconds -from google.cloud._helpers import _microseconds_from_datetime -from google.cloud._helpers import _to_bytes +from google.cloud._helpers import _datetime_from_microseconds # type: ignore +from google.cloud._helpers import _microseconds_from_datetime # type: ignore +from google.cloud._helpers import _to_bytes # type: ignore from google.cloud.bigtable_v2.types import data as data_v2_pb2 diff --git a/google/cloud/bigtable/row_data.py b/google/cloud/bigtable/row_data.py index 18d82153b..6ab1188a8 100644 --- a/google/cloud/bigtable/row_data.py +++ b/google/cloud/bigtable/row_data.py @@ -17,12 +17,12 @@ import copy -import grpc +import grpc # type: ignore from google.api_core import exceptions from google.api_core import retry -from google.cloud._helpers import _datetime_from_microseconds -from google.cloud._helpers import _to_bytes +from google.cloud._helpers import _datetime_from_microseconds # type: ignore +from google.cloud._helpers import _to_bytes # type: ignore from google.cloud.bigtable_v2.types import bigtable as data_messages_v2_pb2 from google.cloud.bigtable_v2.types import data as data_v2_pb2 diff --git a/google/cloud/bigtable/row_filters.py b/google/cloud/bigtable/row_filters.py index b495fb646..53192acc8 100644 --- a/google/cloud/bigtable/row_filters.py +++ b/google/cloud/bigtable/row_filters.py @@ -17,8 +17,8 @@ import struct -from google.cloud._helpers import _microseconds_from_datetime -from google.cloud._helpers import _to_bytes +from google.cloud._helpers import _microseconds_from_datetime # type: ignore +from google.cloud._helpers import _to_bytes # type: ignore from google.cloud.bigtable_v2.types import data as data_v2_pb2 _PACK_I64 = struct.Struct(">q").pack diff --git a/google/cloud/bigtable/row_set.py b/google/cloud/bigtable/row_set.py index 32a9bd1e3..82a540b5a 100644 --- a/google/cloud/bigtable/row_set.py +++ b/google/cloud/bigtable/row_set.py @@ -15,7 +15,7 @@ """User-friendly container for Google Cloud Bigtable RowSet """ -from google.cloud._helpers import _to_bytes +from google.cloud._helpers import _to_bytes # type: ignore class RowSet(object): diff --git a/google/cloud/bigtable/table.py b/google/cloud/bigtable/table.py index 8dc4f5e42..fddd04809 100644 --- a/google/cloud/bigtable/table.py +++ b/google/cloud/bigtable/table.py @@ -14,6 +14,7 @@ """User-friendly container for Google Cloud Bigtable Table.""" +from typing import Set import warnings from google.api_core import timeout @@ -25,7 +26,7 @@ from google.api_core.gapic_v1.method import DEFAULT from google.api_core.retry import if_exception_type from google.api_core.retry import Retry -from google.cloud._helpers import _to_bytes +from google.cloud._helpers import _to_bytes # type: ignore from google.cloud.bigtable.backup import Backup from google.cloud.bigtable.column_family import _gc_rule_from_pb from google.cloud.bigtable.column_family import ColumnFamily @@ -57,6 +58,12 @@ RETRYABLE_MUTATION_ERRORS = (Aborted, DeadlineExceeded, ServiceUnavailable) """Errors which can be retried during row mutation.""" +RETRYABLE_CODES: Set[int] = set() + +for retryable in RETRYABLE_MUTATION_ERRORS: + if retryable.grpc_status_code is not None: # pragma: NO COVER + RETRYABLE_CODES.add(retryable.grpc_status_code.value[0]) + class _BigtableRetryableError(Exception): """Retry-able error expected by the default retry strategy.""" @@ -1043,10 +1050,6 @@ class _RetryableMutateRowsWorker(object): are retryable, any subsequent call on this callable will be a no-op. """ - RETRY_CODES = tuple( - retryable.grpc_status_code.value[0] for retryable in RETRYABLE_MUTATION_ERRORS - ) - def __init__(self, client, table_name, rows, app_profile_id=None, timeout=None): self.client = client self.table_name = table_name @@ -1083,7 +1086,7 @@ def __call__(self, retry=DEFAULT_RETRY): @staticmethod def _is_retryable(status): - return status is None or status.code in _RetryableMutateRowsWorker.RETRY_CODES + return status is None or status.code in RETRYABLE_CODES def _do_mutate_retryable_rows(self): """Mutate all the rows that are eligible for retry. @@ -1128,7 +1131,7 @@ def _do_mutate_retryable_rows(self): **kwargs ) except RETRYABLE_MUTATION_ERRORS: - # If an exception, considered retryable by `RETRY_CODES`, is + # If an exception, considered retryable by `RETRYABLE_MUTATION_ERRORS`, is # returned from the initial call, consider # it to be retryable. Wrap as a Bigtable Retryable Error. raise _BigtableRetryableError diff --git a/mypy.ini b/mypy.ini new file mode 100644 index 000000000..9aef441de --- /dev/null +++ b/mypy.ini @@ -0,0 +1,6 @@ +[mypy] +python_version = 3.6 +namespace_packages = True + +[mypy-google.protobuf] +ignore_missing_imports = True diff --git a/noxfile.py b/noxfile.py index 15117ee22..206e146f4 100644 --- a/noxfile.py +++ b/noxfile.py @@ -38,6 +38,7 @@ "unit", "system_emulated", "system", + "mypy", "cover", "lint", "lint_setup_py", @@ -72,6 +73,15 @@ def blacken(session): ) +@nox.session(python=DEFAULT_PYTHON_VERSION) +def mypy(session): + """Verify type hints are mypy compatible.""" + session.install("-e", ".") + session.install("mypy", "types-setuptools") + # TODO: also verify types on tests, all of google package + session.run("mypy", "-p", "google.cloud.bigtable", "--no-incremental") + + @nox.session(python=DEFAULT_PYTHON_VERSION) def lint_setup_py(session): """Verify that setup.py is valid (including RST check).""" diff --git a/owlbot.py b/owlbot.py index bacfdfbd1..438628413 100644 --- a/owlbot.py +++ b/owlbot.py @@ -149,10 +149,32 @@ def system_emulated(session): """nox.options.sessions = [ "unit", "system_emulated", - "system",""", + "system", + "mypy",""", ) +s.replace( + "noxfile.py", + """\ +@nox.session\(python=DEFAULT_PYTHON_VERSION\) +def lint_setup_py\(session\): +""", + '''\ +@nox.session(python=DEFAULT_PYTHON_VERSION) +def mypy(session): + """Verify type hints are mypy compatible.""" + session.install("-e", ".") + session.install("mypy", "types-setuptools") + # TODO: also verify types on tests, all of google package + session.run("mypy", "-p", "google.cloud.bigtable", "--no-incremental") + + +@nox.session(python=DEFAULT_PYTHON_VERSION) +def lint_setup_py(session): +''', +) + # ---------------------------------------------------------------------------- # Samples templates # ---------------------------------------------------------------------------- diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index b80290e4f..f6cd7a5cc 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -112,7 +112,8 @@ def _make_one(self, *args, **kwargs): @mock.patch("os.environ", {}) def test_constructor_defaults(self): - from google.cloud.bigtable.client import _CLIENT_INFO + from google.api_core import client_info + from google.cloud.bigtable import __version__ from google.cloud.bigtable.client import DATA_SCOPE credentials = _make_credentials() @@ -125,7 +126,8 @@ def test_constructor_defaults(self): self.assertIs(client._credentials, credentials.with_scopes.return_value) self.assertFalse(client._read_only) self.assertFalse(client._admin) - self.assertIs(client._client_info, _CLIENT_INFO) + self.assertIsInstance(client._client_info, client_info.ClientInfo) + self.assertEqual(client._client_info.client_library_version, __version__) self.assertIsNone(client._channel) self.assertIsNone(client._emulator_host) self.assertEqual(client.SCOPE, (DATA_SCOPE,)) @@ -399,7 +401,6 @@ def test_project_path_property(self): self.assertEqual(client.project_path, project_name) def test_table_data_client_not_initialized(self): - from google.cloud.bigtable.client import _CLIENT_INFO from google.cloud.bigtable_v2 import BigtableClient credentials = _make_credentials() @@ -407,7 +408,6 @@ def test_table_data_client_not_initialized(self): table_data_client = client.table_data_client self.assertIsInstance(table_data_client, BigtableClient) - self.assertIs(client._client_info, _CLIENT_INFO) self.assertIs(client._table_data_client, table_data_client) def test_table_data_client_not_initialized_w_client_info(self): @@ -466,7 +466,6 @@ def test_table_admin_client_not_initialized_no_admin_flag(self): client.table_admin_client() def test_table_admin_client_not_initialized_w_admin_flag(self): - from google.cloud.bigtable.client import _CLIENT_INFO from google.cloud.bigtable_admin_v2 import BigtableTableAdminClient credentials = _make_credentials() @@ -476,7 +475,6 @@ def test_table_admin_client_not_initialized_w_admin_flag(self): table_admin_client = client.table_admin_client self.assertIsInstance(table_admin_client, BigtableTableAdminClient) - self.assertIs(client._client_info, _CLIENT_INFO) self.assertIs(client._table_admin_client, table_admin_client) def test_table_admin_client_not_initialized_w_client_info(self): @@ -537,7 +535,6 @@ def test_instance_admin_client_not_initialized_no_admin_flag(self): client.instance_admin_client() def test_instance_admin_client_not_initialized_w_admin_flag(self): - from google.cloud.bigtable.client import _CLIENT_INFO from google.cloud.bigtable_admin_v2 import BigtableInstanceAdminClient credentials = _make_credentials() @@ -547,7 +544,6 @@ def test_instance_admin_client_not_initialized_w_admin_flag(self): instance_admin_client = client.instance_admin_client self.assertIsInstance(instance_admin_client, BigtableInstanceAdminClient) - self.assertIs(client._client_info, _CLIENT_INFO) self.assertIs(client._instance_admin_client, instance_admin_client) def test_instance_admin_client_not_initialized_w_client_info(self):