fix: improve type hints, mypy checks (#448)

* 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 <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Tres Seaver <tseaver@palladion.com>

google/cloud/__init__.py

@@ -1,8 +1,10 @@
+from typing import List
+
 try:
     import pkg_resources
 
     pkg_resources.declare_namespace(__name__)
 except ImportError:
     import pkgutil
 
-    __path__ = pkgutil.extend_path(__path__, __name__)
+    __path__: List[str] = pkgutil.extend_path(__path__, __name__)

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"]

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(

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."

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
 

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"

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
 
 

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
 

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

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):

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

mypy.ini

@@ -0,0 +1,6 @@
+[mypy]
+python_version = 3.6
+namespace_packages = True
+
+[mypy-google.protobuf]
+ignore_missing_imports = True

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)."""

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
 # ----------------------------------------------------------------------------

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,15 +401,13 @@ 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()
         client = self._make_one(project=self.PROJECT, credentials=credentials)
 
         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):