Make Publisher batch-related interfaces private (googleapis#5784)

mcdonc · theacodes · commit 841bcbd6d1ab · 2018-08-10T17:37:27.000-05:00
diff --git a/docs/pubsub/publisher/api/batch.rst b/docs/pubsub/publisher/api/batch.rst
diff --git a/docs/pubsub/publisher/index.rst b/docs/pubsub/publisher/index.rst
@@ -58,23 +58,21 @@ If you want to include attributes, simply add keyword arguments:
 Batching
 --------
 
-Whenever you publish a message, a
-:class:`~.pubsub_v1.publisher.batch.thread.Batch` is automatically created.
-This way, if you publish a large volume of messages, it reduces the number of
-requests made to the server.
+Whenever you publish a message, the publisher will automatically batch the
+messages over a small time window to avoid making too many separate requests to
+the service.  This helps increase throughput.
 
 .. note::
 
     By default, this uses ``threading``, and you will need to be in an
     environment with threading enabled. It is possible to provide an
     alternative batch class that uses another concurrency strategy.
 
-The way that this works is that on the first message that you send, a new
-:class:`~.pubsub_v1.publisher.batch.thread.Batch` is created automatically.
-For every subsequent message, if there is already a valid batch that is still
-accepting messages, then that batch is used. When the batch is created, it
-begins a countdown that publishes the batch once sufficient time has
-elapsed (by default, this is 0.05 seconds).
+The way that this works is that on the first message that you send, a new batch
+is created automatically.  For every subsequent message, if there is already a
+valid batch that is still accepting messages, then that batch is used. When the
+batch is created, it begins a countdown that publishes the batch once
+sufficient time has elapsed (by default, this is 0.05 seconds).
 
 If you need different batching settings, simply provide a
 :class:`~.pubsub_v1.types.BatchSettings` object when you instantiate the
diff --git a/pubsub/google/cloud/pubsub_v1/publisher/_batch/__init__.py b/pubsub/google/cloud/pubsub_v1/publisher/_batch/__init__.py
diff --git a/pubsub/google/cloud/pubsub_v1/publisher/_batch/base.py b/pubsub/google/cloud/pubsub_v1/publisher/_batch/base.py
diff --git a/pubsub/google/cloud/pubsub_v1/publisher/_batch/thread.py b/pubsub/google/cloud/pubsub_v1/publisher/_batch/thread.py
@@ -24,7 +24,7 @@
 from google.cloud.pubsub_v1 import types
 from google.cloud.pubsub_v1.publisher import exceptions
 from google.cloud.pubsub_v1.publisher import futures
-from google.cloud.pubsub_v1.publisher.batch import base
+from google.cloud.pubsub_v1.publisher._batch import base
 
 
 _LOGGER = logging.getLogger(__name__)
diff --git a/pubsub/google/cloud/pubsub_v1/publisher/client.py b/pubsub/google/cloud/pubsub_v1/publisher/client.py
@@ -26,14 +26,15 @@
 from google.cloud.pubsub_v1 import _gapic
 from google.cloud.pubsub_v1 import types
 from google.cloud.pubsub_v1.gapic import publisher_client
-from google.cloud.pubsub_v1.publisher.batch import thread
+from google.cloud.pubsub_v1.publisher._batch import thread
 
 
 __version__ = pkg_resources.get_distribution('google-cloud-pubsub').version
 
 
 @_gapic.add_methods(publisher_client.PublisherClient, blacklist=('publish',))
 class Client(object):
+    _batch_class = thread.Batch
     """A publisher client for Google Cloud Pub/Sub.
 
     This creates an object that is capable of publishing messages.
@@ -43,14 +44,6 @@ class Client(object):
     Args:
         batch_settings (~google.cloud.pubsub_v1.types.BatchSettings): The
             settings for batch publishing.
-        batch_class (Optional[Type]): A class that describes how to handle
-            batches. You may subclass the
-            :class:`.pubsub_v1.publisher.batch.base.BaseBatch` class in
-            order to define your own batcher. This is primarily provided to
-            allow use of different concurrency models; the default
-            is based on :class:`threading.Thread`. This class should also have
-            a class method (or static method) that takes no arguments and
-            produces a lock that can be used as a context manager.
         kwargs (dict): Any additional arguments provided are sent as keyword
             arguments to the underlying
             :class:`~.gapic.pubsub.v1.publisher_client.PublisherClient`.
@@ -59,7 +52,7 @@ class Client(object):
             be added if ``credentials`` are passed explicitly or if the
             Pub / Sub emulator is detected as running.
     """
-    def __init__(self, batch_settings=(), batch_class=thread.Batch, **kwargs):
+    def __init__(self, batch_settings=(), **kwargs):
         # Sanity check: Is our goal to use the emulator?
         # If so, create a grpc insecure channel with the emulator host
         # as the target.
@@ -89,8 +82,7 @@ def __init__(self, batch_settings=(), batch_class=thread.Batch, **kwargs):
 
         # The batches on the publisher client are responsible for holding
         # messages. One batch exists for each topic.
-        self._batch_class = batch_class
-        self._batch_lock = batch_class.make_lock()
+        self._batch_lock = self._batch_class.make_lock()
         self._batches = {}
 
     @property
@@ -102,7 +94,7 @@ def target(self):
         """
         return publisher_client.PublisherClient.SERVICE_ADDRESS
 
-    def batch(self, topic, create=False, autocommit=True):
+    def _batch(self, topic, create=False, autocommit=True):
         """Return the current batch for the provided topic.
 
         This will create a new batch if ``create=True`` or if no batch
@@ -119,7 +111,7 @@ def batch(self, topic, create=False, autocommit=True):
                 might have (e.g. spawning a worker to publish a batch).
 
         Returns:
-            ~.pubsub_v1.batch.Batch: The batch object.
+            ~.pubsub_v1._batch.Batch: The batch object.
         """
         # If there is no matching batch yet, then potentially create one
         # and place it on the batches dictionary.
@@ -201,11 +193,11 @@ def publish(self, topic, data, **attrs):
         message = types.PubsubMessage(data=data, attributes=attrs)
 
         # Delegate the publishing to the batch.
-        batch = self.batch(topic)
+        batch = self._batch(topic)
         future = None
         while future is None:
             future = batch.publish(message)
             if future is None:
-                batch = self.batch(topic, create=True)
+                batch = self._batch(topic, create=True)
 
         return future
diff --git a/pubsub/tests/unit/pubsub_v1/publisher/batch/test_base.py b/pubsub/tests/unit/pubsub_v1/publisher/batch/test_base.py
@@ -19,8 +19,8 @@
 from google.auth import credentials
 from google.cloud.pubsub_v1 import publisher
 from google.cloud.pubsub_v1 import types
-from google.cloud.pubsub_v1.publisher.batch.base import BatchStatus
-from google.cloud.pubsub_v1.publisher.batch.thread import Batch
+from google.cloud.pubsub_v1.publisher._batch.base import BatchStatus
+from google.cloud.pubsub_v1.publisher._batch.thread import Batch
 
 
 def create_batch(status=None, settings=types.BatchSettings()):
diff --git a/pubsub/tests/unit/pubsub_v1/publisher/batch/test_thread.py b/pubsub/tests/unit/pubsub_v1/publisher/batch/test_thread.py
@@ -22,9 +22,9 @@
 from google.cloud.pubsub_v1 import publisher
 from google.cloud.pubsub_v1 import types
 from google.cloud.pubsub_v1.publisher import exceptions
-from google.cloud.pubsub_v1.publisher.batch.base import BatchStatus
-from google.cloud.pubsub_v1.publisher.batch import thread
-from google.cloud.pubsub_v1.publisher.batch.thread import Batch
+from google.cloud.pubsub_v1.publisher._batch.base import BatchStatus
+from google.cloud.pubsub_v1.publisher._batch import thread
+from google.cloud.pubsub_v1.publisher._batch.thread import Batch
 
 
 def create_client():
diff --git a/pubsub/tests/unit/pubsub_v1/publisher/test_publisher_client.py b/pubsub/tests/unit/pubsub_v1/publisher/test_publisher_client.py
@@ -56,7 +56,7 @@ def test_batch_create():
 
     assert len(client._batches) == 0
     topic = 'topic/path'
-    batch = client.batch(topic, autocommit=False)
+    batch = client._batch(topic, autocommit=False)
     assert client._batches == {topic: batch}
 
 
@@ -68,7 +68,7 @@ def test_batch_exists():
     client._batches[topic] = mock.sentinel.batch
 
     # A subsequent request should return the same batch.
-    batch = client.batch(topic, autocommit=False)
+    batch = client._batch(topic, autocommit=False)
     assert batch is mock.sentinel.batch
     assert client._batches == {topic: batch}
 
@@ -81,7 +81,7 @@ def test_batch_create_and_exists():
     client._batches[topic] = mock.sentinel.batch
 
     # A subsequent request should return the same batch.
-    batch = client.batch(topic, create=True, autocommit=False)
+    batch = client._batch(topic, create=True, autocommit=False)
     assert batch is not mock.sentinel.batch
     assert client._batches == {topic: batch}
 
