diff --git a/.gitignore b/.gitignore index 99c3a1444..b4243ced7 100644 --- a/.gitignore +++ b/.gitignore @@ -29,7 +29,6 @@ pip-log.txt .nox .cache .pytest_cache -.pytype # Mac diff --git a/google/cloud/pubsub_v1/_gapic.py b/google/cloud/pubsub_v1/_gapic.py deleted file mode 100644 index e25c1dc6c..000000000 --- a/google/cloud/pubsub_v1/_gapic.py +++ /dev/null @@ -1,74 +0,0 @@ -# Copyright 2019, Google LLC All rights reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -from __future__ import absolute_import - -import functools -from typing import Callable, Container, Type - - -def add_methods( - source_class: Type, denylist: Container[str] = () -) -> Callable[[Type], Type]: - """Add wrapped versions of the `api` member's methods to the class. - - Any methods passed in `denylist` are not added. - Additionally, any methods explicitly defined on the wrapped class are - not added. - """ - - def wrap(wrapped_fx: Callable, lookup_fx: Callable): - """Wrap a GAPIC method; preserve its name and docstring.""" - # If this is a static or class method, then we do *not* - # send self as the first argument. - # - # For instance methods, we need to send self.api rather - # than self, since that is where the actual methods were declared. - - if isinstance(lookup_fx, (classmethod, staticmethod)): - fx = lambda *a, **kw: wrapped_fx(*a, **kw) # noqa - return staticmethod(functools.wraps(wrapped_fx)(fx)) - else: - fx = lambda self, *a, **kw: wrapped_fx(self.api, *a, **kw) # noqa - return functools.wraps(wrapped_fx)(fx) - - def actual_decorator(cls: Type) -> Type: - # Reflectively iterate over most of the methods on the source class - # (the GAPIC) and make wrapped versions available on this client. - for name in dir(source_class): - # Ignore all private and magic methods. - if name.startswith("_"): - continue - - # Ignore anything on our denylist. - if name in denylist: - continue - - # Retrieve the attribute, and ignore it if it is not callable. - attr = getattr(source_class, name) - if not callable(attr): - continue - - # Add a wrapper method to this object. - lookup_fx = source_class.__dict__[name] - fx = wrap(attr, lookup_fx) - - setattr(cls, name, fx) - - # Return the augmented class. - return cls - - # Simply return the actual decorator; this is returned from this method - # and actually used to decorate the class. - return actual_decorator diff --git a/google/cloud/pubsub_v1/publisher/_batch/thread.py b/google/cloud/pubsub_v1/publisher/_batch/thread.py index ade135f45..8b868eaee 100644 --- a/google/cloud/pubsub_v1/publisher/_batch/thread.py +++ b/google/cloud/pubsub_v1/publisher/_batch/thread.py @@ -271,7 +271,7 @@ def _commit(self) -> None: batch_transport_succeeded = True try: # Performs retries for errors defined by the retry configuration. - response = self._client.api.publish( + response = self._client._gapic_publish( topic=self._topic, messages=self._messages, retry=self._commit_retry, diff --git a/google/cloud/pubsub_v1/publisher/client.py b/google/cloud/pubsub_v1/publisher/client.py index 58baf43b6..43305afcc 100644 --- a/google/cloud/pubsub_v1/publisher/client.py +++ b/google/cloud/pubsub_v1/publisher/client.py @@ -22,12 +22,12 @@ import time import typing from typing import Any, Dict, Optional, Sequence, Tuple, Type, Union +import warnings from google.api_core import gapic_v1 from google.auth.credentials import AnonymousCredentials # type: ignore from google.oauth2 import service_account # type: ignore -from google.cloud.pubsub_v1 import _gapic from google.cloud.pubsub_v1 import types from google.cloud.pubsub_v1.publisher import exceptions from google.cloud.pubsub_v1.publisher import futures @@ -49,15 +49,11 @@ from google.cloud import pubsub_v1 from google.cloud.pubsub_v1.publisher import _batch from google.pubsub_v1.services.publisher.client import OptionalRetry + from google.pubsub_v1.types import pubsub as pubsub_types _LOGGER = logging.getLogger(__name__) -_DENYLISTED_METHODS = ( - "publish", - "from_service_account_file", - "from_service_account_json", -) _raw_proto_pubbsub_message = gapic_types.PubsubMessage.pb() @@ -66,8 +62,7 @@ ] -@_gapic.add_methods(publisher_client.PublisherClient, denylist=_DENYLISTED_METHODS) -class Client(object): +class Client(publisher_client.PublisherClient): """A publisher client for Google Cloud Pub/Sub. This creates an object that is capable of publishing messages. @@ -146,8 +141,8 @@ def __init__( # Add the metrics headers, and instantiate the underlying GAPIC # client. - self.api = publisher_client.PublisherClient(**kwargs) - self._target = self.api._transport._host + super().__init__(**kwargs) + self._target = self._transport._host self._batch_class = thread.Batch self.batch_settings = types.BatchSettings(*batch_settings) @@ -164,7 +159,7 @@ def __init__( self._flow_controller = FlowController(self.publisher_options.flow_control) @classmethod - def from_service_account_file( + def from_service_account_file( # type: ignore[override] cls, filename: str, batch_settings: Union[types.BatchSettings, Sequence] = (), @@ -188,7 +183,7 @@ def from_service_account_file( kwargs["credentials"] = credentials return cls(batch_settings, **kwargs) - from_service_account_json = from_service_account_file + from_service_account_json = from_service_account_file # type: ignore[assignment] @property def target(self) -> str: @@ -199,6 +194,26 @@ def target(self) -> str: """ return self._target + @property + def api(self): + """The underlying gapic API client. + + .. versionchanged:: 2.10.0 + Instead of a GAPIC ``PublisherClient`` client instance, this property is a + proxy object to it with the same interface. + + .. deprecated:: 2.10.0 + Use the GAPIC methods and properties on the client instance directly + instead of through the :attr:`api` attribute. + """ + msg = ( + 'The "api" property only exists for backward compatibility, access its ' + 'attributes directly thorugh the client instance (e.g. "client.foo" ' + 'instead of "client.api.foo").' + ) + warnings.warn(msg, category=DeprecationWarning) + return super() + def _get_or_create_sequencer(self, topic: str, ordering_key: str) -> SequencerType: """ Get an existing sequencer or create a new one given the (topic, ordering_key) pair. @@ -252,7 +267,11 @@ def resume_publish(self, topic: str, ordering_key: str) -> None: else: sequencer.unpause() - def publish( + def _gapic_publish(self, *args, **kwargs) -> "pubsub_types.PublishResponse": + """Call the GAPIC public API directly.""" + return super().publish(*args, **kwargs) + + def publish( # type: ignore[override] self, topic: str, data: bytes, @@ -382,7 +401,7 @@ def on_publish_done(future): if self._enable_message_ordering: if retry is gapic_v1.method.DEFAULT: # use the default retry for the publish GRPC method as a base - transport = self.api._transport + transport = self._transport base_retry = transport._wrapped_methods[transport.publish]._retry retry = base_retry.with_deadline(2.0 ** 32) else: diff --git a/google/cloud/pubsub_v1/publisher/flow_controller.py b/google/cloud/pubsub_v1/publisher/flow_controller.py index 3c0558fe5..baf6ba8ff 100644 --- a/google/cloud/pubsub_v1/publisher/flow_controller.py +++ b/google/cloud/pubsub_v1/publisher/flow_controller.py @@ -25,7 +25,7 @@ _LOGGER = logging.getLogger(__name__) -MessageType = Type[types.PubsubMessage] # type: ignore # pytype: disable=module-attr +MessageType = Type[types.PubsubMessage] # type: ignore class _QuantityReservation: diff --git a/google/cloud/pubsub_v1/subscriber/_protocol/leaser.py b/google/cloud/pubsub_v1/subscriber/_protocol/leaser.py index 7cd9317e6..bfa1b5a49 100644 --- a/google/cloud/pubsub_v1/subscriber/_protocol/leaser.py +++ b/google/cloud/pubsub_v1/subscriber/_protocol/leaser.py @@ -76,7 +76,7 @@ def message_count(self) -> int: return len(self._leased_messages) @property - def ack_ids(self) -> KeysView[str]: # pytype: disable=invalid-annotation + def ack_ids(self) -> KeysView[str]: """The ack IDs of all leased messages.""" return self._leased_messages.keys() diff --git a/google/cloud/pubsub_v1/subscriber/_protocol/streaming_pull_manager.py b/google/cloud/pubsub_v1/subscriber/_protocol/streaming_pull_manager.py index 3a2bc6bc1..d207718fc 100644 --- a/google/cloud/pubsub_v1/subscriber/_protocol/streaming_pull_manager.py +++ b/google/cloud/pubsub_v1/subscriber/_protocol/streaming_pull_manager.py @@ -532,7 +532,7 @@ def open( self._get_initial_request, stream_ack_deadline_seconds ) self._rpc = bidi.ResumableBidiRpc( - start_rpc=self._client.api.streaming_pull, + start_rpc=self._client.streaming_pull, initial_request=get_initial_request, should_recover=self._should_recover, should_terminate=self._should_terminate, @@ -548,14 +548,11 @@ def open( # Create references to threads assert self._scheduler is not None - # pytype: disable=wrong-arg-types - # (pytype incorrectly complains about "self" not being the right argument type) scheduler_queue = self._scheduler.queue self._dispatcher = dispatcher.Dispatcher(self, scheduler_queue) self._consumer = bidi.BackgroundConsumer(self._rpc, self._on_response) self._leaser = leaser.Leaser(self) self._heartbeater = heartbeater.Heartbeater(self) - # pytype: enable=wrong-arg-types # Start the thread to pass the requests. self._dispatcher.start() diff --git a/google/cloud/pubsub_v1/subscriber/client.py b/google/cloud/pubsub_v1/subscriber/client.py index 8eb1e2e25..9c12a0bfb 100644 --- a/google/cloud/pubsub_v1/subscriber/client.py +++ b/google/cloud/pubsub_v1/subscriber/client.py @@ -18,11 +18,11 @@ import pkg_resources import typing from typing import cast, Any, Callable, Optional, Sequence, Union +import warnings from google.auth.credentials import AnonymousCredentials # type: ignore from google.oauth2 import service_account # type: ignore -from google.cloud.pubsub_v1 import _gapic from google.cloud.pubsub_v1 import types from google.cloud.pubsub_v1.subscriber import futures from google.cloud.pubsub_v1.subscriber._protocol import streaming_pull_manager @@ -42,15 +42,8 @@ # a PIP package. __version__ = "0.0" -_DENYLISTED_METHODS = ( - "publish", - "from_service_account_file", - "from_service_account_json", -) - -@_gapic.add_methods(subscriber_client.SubscriberClient, denylist=_DENYLISTED_METHODS) -class Client(object): +class Client(subscriber_client.SubscriberClient): """A subscriber client for Google Cloud Pub/Sub. This creates an object that is capable of subscribing to messages. @@ -91,12 +84,14 @@ def __init__(self, **kwargs: Any): kwargs["credentials"] = AnonymousCredentials() # Instantiate the underlying GAPIC client. - self._api = subscriber_client.SubscriberClient(**kwargs) - self._target = self._api._transport._host + super().__init__(**kwargs) + self._target = self._transport._host self._closed = False @classmethod - def from_service_account_file(cls, filename: str, **kwargs: Any) -> "Client": + def from_service_account_file( # type: ignore[override] + cls, filename: str, **kwargs: Any + ) -> "Client": """Creates an instance of this client using the provided credentials file. @@ -112,7 +107,7 @@ def from_service_account_file(cls, filename: str, **kwargs: Any) -> "Client": kwargs["credentials"] = credentials return cls(**kwargs) - from_service_account_json = from_service_account_file + from_service_account_json = from_service_account_file # type: ignore[assignment] @property def target(self) -> str: @@ -123,11 +118,6 @@ def target(self) -> str: """ return self._target - @property - def api(self) -> subscriber_client.SubscriberClient: - """The underlying gapic API client.""" - return self._api - @property def closed(self) -> bool: """Return whether the client has been closed and cannot be used anymore. @@ -136,6 +126,26 @@ def closed(self) -> bool: """ return self._closed + @property + def api(self): + """The underlying gapic API client. + + .. versionchanged:: 2.10.0 + Instead of a GAPIC ``SubscriberClient`` client instance, this property is a + proxy object to it with the same interface. + + .. deprecated:: 2.10.0 + Use the GAPIC methods and properties on the client instance directly + instead of through the :attr:`api` attribute. + """ + msg = ( + 'The "api" property only exists for backward compatibility, access its ' + 'attributes directly thorugh the client instance (e.g. "client.foo" ' + 'instead of "client.api.foo").' + ) + warnings.warn(msg, category=DeprecationWarning) + return super() + def subscribe( self, subscription: str, @@ -266,7 +276,7 @@ def close(self) -> None: This method is idempotent. """ - transport = cast("SubscriberGrpcTransport", self.api._transport) + transport = cast("SubscriberGrpcTransport", self._transport) transport.grpc_channel.close() self._closed = True diff --git a/google/cloud/pubsub_v1/subscriber/message.py b/google/cloud/pubsub_v1/subscriber/message.py index 5bd84e9ad..2d72bba57 100644 --- a/google/cloud/pubsub_v1/subscriber/message.py +++ b/google/cloud/pubsub_v1/subscriber/message.py @@ -81,7 +81,7 @@ class Message(object): The time that this message was originally published. """ - def __init__( # pytype: disable=module-attr + def __init__( self, message: "types.PubsubMessage._meta._pb", # type: ignore ack_id: str, diff --git a/google/cloud/pubsub_v1/types.py b/google/cloud/pubsub_v1/types.py index 62dffcfc3..e843a6da9 100644 --- a/google/cloud/pubsub_v1/types.py +++ b/google/cloud/pubsub_v1/types.py @@ -127,13 +127,11 @@ class PublisherOptions(NamedTuple): "an instance of :class:`google.api_core.retry.Retry`." ) - # pytype: disable=invalid-annotation timeout: "OptionalTimeout" = gapic_v1.method.DEFAULT # use api_core default ( "Timeout settings for message publishing by the client. It should be " "compatible with :class:`~.pubsub_v1.types.TimeoutType`." ) - # pytype: enable=invalid-annotation # Define the type class and default values for flow control settings. diff --git a/noxfile.py b/noxfile.py index f5d3cd26c..e9fea8af8 100644 --- a/noxfile.py +++ b/noxfile.py @@ -28,8 +28,6 @@ BLACK_PATHS = ["docs", "google", "tests", "noxfile.py", "setup.py"] MYPY_VERSION = "mypy==0.910" -PYTYPE_VERSION = "pytype==2021.4.9" - DEFAULT_PYTHON_VERSION = "3.8" SYSTEM_TEST_PYTHON_VERSIONS = ["3.10"] @@ -46,8 +44,8 @@ "lint_setup_py", "blacken", "mypy", - "pytype", - # "mypy_samples", # TODO: uncomment when the checks pass + # https://github.com/googleapis/python-pubsub/pull/552#issuecomment-1016256936 + # "mypy_samples", # TODO: uncomment when the check passes "docs", ] @@ -77,14 +75,6 @@ def mypy(session): session.run("mypy", "google/cloud") -@nox.session(python=DEFAULT_PYTHON_VERSION) -def pytype(session): - """Run type checks.""" - session.install("-e", ".[all]") - session.install(PYTYPE_VERSION) - session.run("pytype") - - @nox.session(python=DEFAULT_PYTHON_VERSION) def mypy_samples(session): """Run type checks with mypy.""" diff --git a/owlbot.py b/owlbot.py index 583e82aa9..a58470e65 100644 --- a/owlbot.py +++ b/owlbot.py @@ -364,61 +364,6 @@ # ---------------------------------------------------------------------------- python.py_samples() -# ---------------------------------------------------------------------------- -# pytype-related changes -# ---------------------------------------------------------------------------- - -# Add .pytype to .gitignore -s.replace(".gitignore", r"\.pytest_cache", "\g<0>\n.pytype") - -# Add pytype config to setup.cfg -s.replace( - "setup.cfg", - r"universal = 1", - textwrap.dedent( - """ \g<0> - - [pytype] - python_version = 3.8 - inputs = - google/cloud/ - exclude = - tests/ - google/pubsub_v1/ # generated code - output = .pytype/ - disable = - # There's some issue with finding some pyi files, thus disabling. - # The issue https://github.com/google/pytype/issues/150 is closed, but the - # error still occurs for some reason. - pyi-error""" - ), -) - -# Add pytype session to noxfile.py -s.replace( - "noxfile.py", - r"BLACK_PATHS = \[.*?\]", - '\g<0>\nPYTYPE_VERSION = "pytype==2021.4.9"\n', -) -s.replace( - "noxfile.py", r'"blacken",', '\g<0>\n "pytype",', -) -s.replace( - "noxfile.py", - r"nox\.options\.error_on_missing_interpreters = True", - textwrap.dedent( - ''' \g<0> - - - @nox.session(python=DEFAULT_PYTHON_VERSION) - def pytype(session): - """Run type checks.""" - session.install("-e", ".[all]") - session.install(PYTYPE_VERSION) - session.run("pytype")''' - ), -) - # ---------------------------------------------------------------------------- # Add mypy nox session. # ---------------------------------------------------------------------------- @@ -467,12 +412,13 @@ def mypy(session): # ---------------------------------------------------------------------------- s.replace( "noxfile.py", - r'"pytype",', - '\g<0>\n # "mypy_samples", # TODO: uncomment when the checks pass', + r' "mypy",', + '\g<0>\n # https://github.com/googleapis/python-pubsub/pull/552#issuecomment-1016256936' + '\n # "mypy_samples", # TODO: uncomment when the check passes', ) s.replace( "noxfile.py", - r'session\.run\("pytype"\)', + r'session\.run\("mypy", "google/cloud"\)', textwrap.dedent( ''' \g<0> diff --git a/samples/snippets/subscriber.py b/samples/snippets/subscriber.py index 955dd278f..f44f82c4a 100644 --- a/samples/snippets/subscriber.py +++ b/samples/snippets/subscriber.py @@ -332,11 +332,8 @@ def update_subscription_with_dead_letter_policy( ) with subscriber: - subscription_after_update = typing.cast( - "gapic_types.Subscription", - subscriber.update_subscription( - request={"subscription": subscription, "update_mask": update_mask} - ), + subscription_after_update = subscriber.update_subscription( + request={"subscription": subscription, "update_mask": update_mask} ) print(f"After the update: {subscription_after_update}.") @@ -380,11 +377,8 @@ def remove_dead_letter_policy( ) with subscriber: - subscription_after_update = typing.cast( - "gapic_types.Subscription", - subscriber.update_subscription( - request={"subscription": subscription, "update_mask": update_mask} - ), + subscription_after_update = subscriber.update_subscription( + request={"subscription": subscription, "update_mask": update_mask} ) print(f"After removing the policy: {subscription_after_update}.") diff --git a/setup.cfg b/setup.cfg index a79cb6a60..c3a2b39f6 100644 --- a/setup.cfg +++ b/setup.cfg @@ -17,17 +17,3 @@ # Generated by synthtool. DO NOT EDIT! [bdist_wheel] universal = 1 - -[pytype] -python_version = 3.8 -inputs = - google/cloud/ -exclude = - tests/ - google/pubsub_v1/ # generated code -output = .pytype/ -disable = - # There's some issue with finding some pyi files, thus disabling. - # The issue https://github.com/google/pytype/issues/150 is closed, but the - # error still occurs for some reason. - pyi-error diff --git a/tests/unit/pubsub_v1/publisher/batch/test_thread.py b/tests/unit/pubsub_v1/publisher/batch/test_thread.py index abf5ec76f..b15128489 100644 --- a/tests/unit/pubsub_v1/publisher/batch/test_thread.py +++ b/tests/unit/pubsub_v1/publisher/batch/test_thread.py @@ -128,7 +128,7 @@ def test_blocking__commit(): # Set up the underlying API publish method to return a PublishResponse. publish_response = gapic_types.PublishResponse(message_ids=["a", "b"]) patch = mock.patch.object( - type(batch.client.api), "publish", return_value=publish_response + type(batch.client), "_gapic_publish", return_value=publish_response ) with patch as publish: batch._commit() @@ -160,7 +160,7 @@ def test_blocking__commit_custom_retry(): # Set up the underlying API publish method to return a PublishResponse. publish_response = gapic_types.PublishResponse(message_ids=["a"]) patch = mock.patch.object( - type(batch.client.api), "publish", return_value=publish_response + type(batch.client), "_gapic_publish", return_value=publish_response ) with patch as publish: batch._commit() @@ -182,7 +182,7 @@ def test_blocking__commit_custom_timeout(): # Set up the underlying API publish method to return a PublishResponse. publish_response = gapic_types.PublishResponse(message_ids=["a"]) patch = mock.patch.object( - type(batch.client.api), "publish", return_value=publish_response + type(batch.client), "_gapic_publish", return_value=publish_response ) with patch as publish: batch._commit() @@ -208,7 +208,7 @@ def api_publish_delay(topic="", messages=(), retry=None, timeout=None): return gapic_types.PublishResponse(message_ids=message_ids) api_publish_patch = mock.patch.object( - type(batch.client.api), "publish", side_effect=api_publish_delay + type(batch.client), "_gapic_publish", side_effect=api_publish_delay ) with api_publish_patch: @@ -252,7 +252,7 @@ def test_blocking__commit_already_started(_LOGGER): def test_blocking__commit_no_messages(): batch = create_batch() - with mock.patch.object(type(batch.client.api), "publish") as publish: + with mock.patch.object(type(batch.client), "_gapic_publish") as publish: batch._commit() assert publish.call_count == 0 @@ -268,7 +268,7 @@ def test_blocking__commit_wrong_messageid_length(): # Set up a PublishResponse that only returns one message ID. publish_response = gapic_types.PublishResponse(message_ids=["a"]) patch = mock.patch.object( - type(batch.client.api), "publish", return_value=publish_response + type(batch.client), "_gapic_publish", return_value=publish_response ) with patch: @@ -288,7 +288,7 @@ def test_block__commmit_api_error(): # Make the API throw an error when publishing. error = google.api_core.exceptions.InternalServerError("uh oh") - patch = mock.patch.object(type(batch.client.api), "publish", side_effect=error) + patch = mock.patch.object(type(batch.client), "_gapic_publish", side_effect=error) with patch: batch._commit() @@ -307,7 +307,7 @@ def test_block__commmit_retry_error(): # Make the API throw an error when publishing. error = google.api_core.exceptions.RetryError("uh oh", None) - patch = mock.patch.object(type(batch.client.api), "publish", side_effect=error) + patch = mock.patch.object(type(batch.client), "_gapic_publish", side_effect=error) with patch: batch._commit() @@ -536,7 +536,7 @@ def test_batch_done_callback_called_on_success(): publish_response = gapic_types.PublishResponse(message_ids=["a"]) with mock.patch.object( - type(batch.client.api), "publish", return_value=publish_response + type(batch.client), "_gapic_publish", return_value=publish_response ): batch._commit() @@ -559,8 +559,8 @@ def test_batch_done_callback_called_on_publish_failure(): error = google.api_core.exceptions.InternalServerError("uh oh") with mock.patch.object( - type(batch.client.api), - "publish", + type(batch.client), + "_gapic_publish", return_value=publish_response, side_effect=error, ): @@ -582,7 +582,7 @@ def test_batch_done_callback_called_on_publish_response_invalid(): publish_response = gapic_types.PublishResponse(message_ids=[]) with mock.patch.object( - type(batch.client.api), "publish", return_value=publish_response + type(batch.client), "_gapic_publish", return_value=publish_response ): batch._commit() diff --git a/tests/unit/pubsub_v1/publisher/test_publisher_client.py b/tests/unit/pubsub_v1/publisher/test_publisher_client.py index 161f9e33b..20d5b328c 100644 --- a/tests/unit/pubsub_v1/publisher/test_publisher_client.py +++ b/tests/unit/pubsub_v1/publisher/test_publisher_client.py @@ -22,6 +22,7 @@ import mock import pytest import time +import warnings from google.api_core import gapic_v1 from google.api_core import retry as retries @@ -50,12 +51,42 @@ def _assert_retries_equal(retry, retry2): assert inspect.getclosurevars(pred) == inspect.getclosurevars(pred2) +def test_api_property_deprecated(creds): + client = publisher.Client(credentials=creds) + + with warnings.catch_warnings(record=True) as warned: + client.api + + assert len(warned) == 1 + assert issubclass(warned[0].category, DeprecationWarning) + warning_msg = str(warned[0].message) + assert "client.api" in warning_msg + + +def test_api_property_proxy_to_generated_client(creds): + client = publisher.Client(credentials=creds) + + with warnings.catch_warnings(record=True): + api_object = client.api + + # Not a perfect check, but we are satisficed if the returned API object indeed + # contains all methods of the generated class. + superclass_attrs = (attr for attr in dir(type(client).__mro__[1])) + assert all( + hasattr(api_object, attr) + for attr in superclass_attrs + if callable(getattr(client, attr)) + ) + + # The resume_publish() method only exists on the hand-written wrapper class. + assert hasattr(client, "resume_publish") + assert not hasattr(api_object, "resume_publish") + + def test_init(creds): client = publisher.Client(credentials=creds) - # A plain client should have an `api` (the underlying GAPIC) and a - # batch settings object, which should have the defaults. - assert isinstance(client.api, publisher_client.PublisherClient) + # A plain client should have a batch settings object containing the defaults. assert client.batch_settings.max_bytes == 1 * 1000 * 1000 assert client.batch_settings.max_latency == 0.01 assert client.batch_settings.max_messages == 100 @@ -67,7 +98,7 @@ def test_init_default_client_info(creds): installed_version = publisher.client.__version__ expected_client_info = f"gccl/{installed_version}" - for wrapped_method in client.api.transport._wrapped_methods.values(): + for wrapped_method in client.transport._wrapped_methods.values(): user_agent = next( ( header_value @@ -84,10 +115,10 @@ def test_init_w_custom_transport(creds): transport = PublisherGrpcTransport(credentials=creds) client = publisher.Client(transport=transport) - # A plain client should have an `api` (the underlying GAPIC) and a - # batch settings object, which should have the defaults. - assert isinstance(client.api, publisher_client.PublisherClient) - assert client.api._transport is transport + # A plain client should have a transport and a batch settings object, which should + # contain the defaults. + assert isinstance(client, publisher_client.PublisherClient) + assert client._transport is transport assert client.batch_settings.max_bytes == 1 * 1000 * 1000 assert client.batch_settings.max_latency == 0.01 assert client.batch_settings.max_messages == 100 @@ -97,8 +128,7 @@ def test_init_w_api_endpoint(creds): client_options = {"api_endpoint": "testendpoint.google.com"} client = publisher.Client(client_options=client_options, credentials=creds) - assert isinstance(client.api, publisher_client.PublisherClient) - assert (client.api._transport.grpc_channel._channel.target()).decode( + assert (client._transport.grpc_channel._channel.target()).decode( "utf-8" ) == "testendpoint.google.com:443" @@ -106,8 +136,7 @@ def test_init_w_api_endpoint(creds): def test_init_w_empty_client_options(creds): client = publisher.Client(client_options={}, credentials=creds) - assert isinstance(client.api, publisher_client.PublisherClient) - assert (client.api._transport.grpc_channel._channel.target()).decode( + assert (client._transport.grpc_channel._channel.target()).decode( "utf-8" ) == publisher_client.PublisherClient.SERVICE_ADDRESS @@ -129,12 +158,12 @@ def init(self, *args, **kwargs): "credentials_file": "file.json", } ) - client_options = client.api.kwargs["client_options"] + client_options = client.kwargs["client_options"] assert client_options.get("quota_project_id") == "42" assert client_options.get("scopes") == [] assert client_options.get("credentials_file") == "file.json" assert client.target == "testendpoint.google.com" - assert client.api.transport._ssl_channel_credentials == mock_ssl_creds + assert client.transport._ssl_channel_credentials == mock_ssl_creds def test_init_emulator(monkeypatch): @@ -147,7 +176,7 @@ def test_init_emulator(monkeypatch): # # Sadly, there seems to be no good way to do this without poking at # the private API of gRPC. - channel = client.api._transport.publish._channel + channel = client._transport.publish._channel assert channel.target().decode("utf8") == "/foo/bar:123" @@ -418,7 +447,7 @@ def test_gapic_instance_method(creds): transport_mock._wrapped_methods = { transport_mock.create_topic: fake_create_topic_rpc } - patcher = mock.patch.object(client.api, "_transport", new=transport_mock) + patcher = mock.patch.object(client, "_transport", new=transport_mock) topic = gapic_types.Topic(name="projects/foo/topics/bar") diff --git a/tests/unit/pubsub_v1/subscriber/test_streaming_pull_manager.py b/tests/unit/pubsub_v1/subscriber/test_streaming_pull_manager.py index 609026598..42c14c47d 100644 --- a/tests/unit/pubsub_v1/subscriber/test_streaming_pull_manager.py +++ b/tests/unit/pubsub_v1/subscriber/test_streaming_pull_manager.py @@ -553,7 +553,7 @@ def test_open(heartbeater, dispatcher, leaser, background_consumer, resumable_bi assert manager._consumer == background_consumer.return_value resumable_bidi_rpc.assert_called_once_with( - start_rpc=manager._client.api.streaming_pull, + start_rpc=manager._client.streaming_pull, initial_request=mock.ANY, should_recover=manager._should_recover, should_terminate=manager._should_terminate, @@ -562,7 +562,7 @@ def test_open(heartbeater, dispatcher, leaser, background_consumer, resumable_bi initial_request_arg = resumable_bidi_rpc.call_args.kwargs["initial_request"] assert initial_request_arg.func == manager._get_initial_request assert initial_request_arg.args[0] == 18 - assert not manager._client.api.get_subscription.called + assert not manager._client.get_subscription.called resumable_bidi_rpc.return_value.add_done_callback.assert_called_once_with( manager._on_rpc_done diff --git a/tests/unit/pubsub_v1/subscriber/test_subscriber_client.py b/tests/unit/pubsub_v1/subscriber/test_subscriber_client.py index 601b40bcc..1f60b536d 100644 --- a/tests/unit/pubsub_v1/subscriber/test_subscriber_client.py +++ b/tests/unit/pubsub_v1/subscriber/test_subscriber_client.py @@ -26,18 +26,13 @@ from google.pubsub_v1.services.subscriber.transports.grpc import SubscriberGrpcTransport -def test_init(creds): - client = subscriber.Client(credentials=creds) - assert isinstance(client.api, subscriber_client.SubscriberClient) - - def test_init_default_client_info(creds): client = subscriber.Client(credentials=creds) installed_version = subscriber.client.__version__ expected_client_info = f"gccl/{installed_version}" - for wrapped_method in client.api.transport._wrapped_methods.values(): + for wrapped_method in client.transport._wrapped_methods.values(): user_agent = next( ( header_value @@ -58,16 +53,14 @@ def test_init_default_closed_state(creds): def test_init_w_custom_transport(creds): transport = SubscriberGrpcTransport(credentials=creds) client = subscriber.Client(transport=transport) - assert isinstance(client.api, subscriber_client.SubscriberClient) - assert client.api._transport is transport + assert client._transport is transport def test_init_w_api_endpoint(creds): client_options = {"api_endpoint": "testendpoint.google.com"} client = subscriber.Client(client_options=client_options, credentials=creds) - assert isinstance(client.api, subscriber_client.SubscriberClient) - assert (client.api._transport.grpc_channel._channel.target()).decode( + assert (client._transport.grpc_channel._channel.target()).decode( "utf-8" ) == "testendpoint.google.com:443" @@ -75,8 +68,7 @@ def test_init_w_api_endpoint(creds): def test_init_w_empty_client_options(creds): client = subscriber.Client(client_options={}, credentials=creds) - assert isinstance(client.api, subscriber_client.SubscriberClient) - assert (client.api._transport.grpc_channel._channel.target()).decode( + assert (client._transport.grpc_channel._channel.target()).decode( "utf-8" ) == subscriber_client.SubscriberClient.SERVICE_ADDRESS @@ -98,12 +90,12 @@ def init(self, *args, **kwargs): "credentials_file": "file.json", } ) - client_options = client._api.kwargs["client_options"] + client_options = client.kwargs["client_options"] assert client_options.get("quota_project_id") == "42" assert client_options.get("scopes") == [] assert client_options.get("credentials_file") == "file.json" assert client.target == "testendpoint.google.com" - assert client.api.transport._ssl_channel_credentials == mock_ssl_creds + assert client.transport._ssl_channel_credentials == mock_ssl_creds def test_init_emulator(monkeypatch): @@ -116,7 +108,7 @@ def test_init_emulator(monkeypatch): # # Sadly, there seems to be no good way to do this without poking at # the private API of gRPC. - channel = client.api._transport.pull._channel + channel = client._transport.pull._channel assert channel.target().decode("utf8") == "/baz/bacon:123" @@ -184,7 +176,7 @@ def test_subscribe_options(manager_open, creds): def test_close(creds): client = subscriber.Client(credentials=creds) - patcher = mock.patch.object(client.api._transport.grpc_channel, "close") + patcher = mock.patch.object(client._transport.grpc_channel, "close") with patcher as patched_close: client.close() @@ -195,7 +187,7 @@ def test_close(creds): def test_closes_channel_as_context_manager(creds): client = subscriber.Client(credentials=creds) - patcher = mock.patch.object(client.api._transport.grpc_channel, "close") + patcher = mock.patch.object(client._transport.grpc_channel, "close") with patcher as patched_close: with client: @@ -207,7 +199,7 @@ def test_closes_channel_as_context_manager(creds): def test_context_manager_raises_if_closed(creds): client = subscriber.Client(credentials=creds) - with mock.patch.object(client.api._transport.grpc_channel, "close"): + with mock.patch.object(client._transport.grpc_channel, "close"): client.close() expetect_msg = r"(?i).*closed.*cannot.*context manager.*" @@ -216,13 +208,45 @@ def test_context_manager_raises_if_closed(creds): pass +def test_api_property_deprecated(creds): + client = subscriber.Client(credentials=creds) + + with warnings.catch_warnings(record=True) as warned: + client.api + + assert len(warned) == 1 + assert issubclass(warned[0].category, DeprecationWarning) + warning_msg = str(warned[0].message) + assert "client.api" in warning_msg + + +def test_api_property_proxy_to_generated_client(creds): + client = subscriber.Client(credentials=creds) + + with warnings.catch_warnings(record=True): + api_object = client.api + + # Not a perfect check, but we are satisficed if the returned API object indeed + # contains all methods of the generated class. + superclass_attrs = (attr for attr in dir(type(client).__mro__[1])) + assert all( + hasattr(api_object, attr) + for attr in superclass_attrs + if callable(getattr(client, attr)) + ) + + # The close() method only exists on the hand-written wrapper class. + assert hasattr(client, "close") + assert not hasattr(api_object, "close") + + def test_streaming_pull_gapic_monkeypatch(creds): client = subscriber.Client(credentials=creds) with mock.patch("google.api_core.gapic_v1.method.wrap_method"): client.streaming_pull(requests=iter([])) - transport = client.api._transport + transport = client._transport assert hasattr(transport.streaming_pull, "_prefetch_first_result_") assert not transport.streaming_pull._prefetch_first_result_ @@ -232,7 +256,7 @@ def test_sync_pull_warning_if_return_immediately(creds): subscription_path = "projects/foo/subscriptions/bar" with mock.patch.object( - client.api._transport, "_wrapped_methods" + client._transport, "_wrapped_methods" ), warnings.catch_warnings(record=True) as warned: client.pull(subscription=subscription_path, return_immediately=True) diff --git a/tests/unit/pubsub_v1/test__gapic.py b/tests/unit/pubsub_v1/test__gapic.py deleted file mode 100644 index adff4bbfc..000000000 --- a/tests/unit/pubsub_v1/test__gapic.py +++ /dev/null @@ -1,63 +0,0 @@ -# Copyright 2019 Google LLC -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# https://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - - -from google.cloud.pubsub_v1 import _gapic - - -class SourceClass(object): - def __init__(self): - self.x = "x" - - def method(self): - return "source class instance method" - - @staticmethod - def static_method(): - return "source class static method" - - @classmethod - def class_method(cls): - return "source class class method" - - @classmethod - def denylisted_method(cls): # pragma: NO COVER - return "source class denylisted method" - - -def test_add_method(): - @_gapic.add_methods(SourceClass, ("denylisted_method",)) - class Foo(object): - def __init__(self): - self.api = SourceClass() - - def method(self): # pragma: NO COVER - return "foo class instance method" - - foo = Foo() - - # Any method that's callable and not denylisted is "inherited". - assert set(["method", "static_method", "class_method"]) <= set(dir(foo)) - assert "denylisted_method" not in dir(foo) - - # Source Class's static and class methods become static methods. - assert type(Foo.__dict__["static_method"]) == staticmethod - assert foo.static_method() == "source class static method" - assert type(Foo.__dict__["class_method"]) == staticmethod - assert foo.class_method() == "source class class method" - - # The decorator changes the behavior of instance methods of the wrapped class. - # method() is called on an instance of the Source Class (stored as an - # attribute on the wrapped class). - assert foo.method() == "source class instance method"