FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

PubSub Publisher by lukesneeringer · Pull Request #3463 · googleapis/google-cloud-python · GitHub

/ google-cloud-python Public

PubSub Publisher - #3463

Merged
lukesneeringer merged 44 commits into
pubsubfrom
pubsub-publisher
Aug 24, 2017
ConversationCommits (44)ChecksFiles changed
Merged

PubSub Publisher#3463
lukesneeringer merged 44 commits into
pubsubfrom
pubsub-publisher

Conversation

lukesneeringer commented Jun 1, 2017

Copy link
Copy Markdown
Contributor

@jonparrott @dhermes

This is what I assert to be a mostly complete publisher for Pub/Sub (minus error checking, which is a pretty big caveat, and tests, which is also a pretty big caveat). I would like feedback on this much before I move forward, since (as we all know) multiprocessing is tricky.

I am about to add some notes inline at specific places, but obviously feel free to comment on any of it.

lukesneeringer added api: pubsub Issues related to the Pub/Sub API. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. do not merge Indicates a pull request not ready for merge, due to either quality or timing. labels Jun 1, 2017
lukesneeringer self-assigned this Jun 1, 2017
googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 1, 2017
lukesneeringer commented Jun 1, 2017
View reviewed changes
Comment thread pubsub/google/cloud/pubsub_v1/publisher/batch.py Outdated
def __init__(self, client, topic, settings, autocommit=True):
self._client = client

# Create a namespace that is owned by the client manager; this

This comment was marked as spam.

Sign in to view

lukesneeringer commented Jun 1, 2017
View reviewed changes
Comment thread pubsub/google/cloud/pubsub_v1/publisher/batch.py Outdated
raise Exception('Empty queue')
response = self._client.api.publish(self._.topic, self._.messages)

# FIXME (lukesneeringer): Check for failures; retry.

This comment was marked as spam.

Sign in to view

lukesneeringer commented Jun 1, 2017
View reviewed changes
Comment thread pubsub/google/cloud/pubsub_v1/publisher/exceptions.py
@@ -1,4 +1,4 @@
# Copyright 2016 Google Inc.

This comment was marked as spam.

Sign in to view

lukesneeringer commented Jun 1, 2017
View reviewed changes
Comment thread pubsub/google/cloud/pubsub_v1/publisher/future.py
Args:
message_id (str): The message ID, as a string.
"""
for callback in self._callbacks:

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

lukesneeringer commented Jun 1, 2017
View reviewed changes
Comment thread pubsub/google/cloud/pubsub_v1/retry.py Outdated
# See the License for the specific language governing permissions and
# limitations under the License.

def retry(func, delay=0, count=0, err=None, **kwargs):

This comment was marked as spam.

Sign in to view

lukesneeringer commented Jun 1, 2017
View reviewed changes
Comment thread pubsub/google/cloud/pubsub_v1/subscriber.py Outdated
__VERSION__ = pkg_resources.get_distribution('google-cloud-pubsub').version


class SubscriberClient(subscriber_client.SubscriberClient):

This comment was marked as spam.

Sign in to view

theacodes suggested changes Jun 1, 2017
View reviewed changes
Comment thread pubsub/google/cloud/pubsub_v1/__init__.py

__all__ = (
'PublisherClient',
'SubscriberClient',

This comment was marked as spam.

Sign in to view

Comment thread pubsub/google/cloud/pubsub_v1/publisher/batch.py Outdated
:class:`google.cloud.pubsub_v1.PublisherClient`.

Args:
client (:class:`google.cloud.pubsub_v1.PublisherClient`): The

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread pubsub/google/cloud/pubsub_v1/publisher/batch.py Outdated
inferred from this.
topic (str): The topic. The format for this is
``projects/{project}/topics/{topic}``.
settings (:class:`google.cloud.pubsub_v1.types.Batching`): The

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread pubsub/google/cloud/pubsub_v1/publisher/batch.py Outdated
# Create a namespace that is owned by the client manager; this
# is necessary to be able to have these values be communicable between
# processes.
self._ = self.manager.Namespace()

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread pubsub/google/cloud/pubsub_v1/publisher/batch.py Outdated
self._.messages = self.manager.list()
self._.message_ids = self.manager.dict()
self._.settings = settings
self._.status = 'accepting messages'

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread pubsub/google/cloud/pubsub_v1/publisher/batch.py Outdated
Future: An object conforming to the ``concurrent.futures.Future``
interface.
"""
# Sanity check: Is the data being sent as a bytestring?

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread pubsub/google/cloud/pubsub_v1/publisher/batch.py Outdated
return f


# Make a fake batch. This is used by the client to do single-op checks

This comment was marked as spam.

Sign in to view

Comment thread pubsub/google/cloud/pubsub_v1/publisher/batch.py Outdated
def commit(self):
"""Actually publish all of the messages on the active batch.

This moves the batch out from being the active batch to an in-flight

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread pubsub/google/cloud/pubsub_v1/publisher/future.py
Args:
message_id (str): The message ID, as a string.
"""
for callback in self._callbacks:

This comment was marked as spam.

Sign in to view

dhermes reviewed Jun 1, 2017
View reviewed changes
Comment thread pubsub/google/cloud/pubsub_v1/_gapic.py Outdated
import functools


def add_methods(SourceClass, blacklist=()):

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread pubsub/google/cloud/pubsub_v1/publisher/batch.py Outdated
:class:`google.cloud.pubsub_v1.PublisherClient`.

Args:
client (:class:`google.cloud.pubsub_v1.PublisherClient`): The

This comment was marked as spam.

Sign in to view

Comment thread pubsub/google/cloud/pubsub_v1/publisher/batch.py Outdated
message to be published is received; subsequent messages are added to
that batch until the process of actual publishing _starts_.

Once this occurs, any new messages sent to ``publish`` open a new batch.

This comment was marked as spam.

Sign in to view

Comment thread pubsub/google/cloud/pubsub_v1/publisher/batch.py Outdated
Future: An object conforming to the ``concurrent.futures.Future``
interface.
"""
# Sanity check: Is the data being sent as a bytestring?

This comment was marked as spam.

Sign in to view

Comment thread pubsub/google/cloud/pubsub_v1/publisher/client.py Outdated
:class:`~gapic.pubsub.v1.publisher_client.PublisherClient`.
Generally, you should not need to set additional keyword arguments.
"""
_gapic_class = publisher_client.PublisherClient

This comment was marked as spam.

Sign in to view

Comment thread pubsub/google/cloud/pubsub_v1/publisher/client.py Outdated
"""
# If there is no matching batch yet, then potentially create one
# and place it on the batches dictionary.
if self._batches.get(topic, FAKE).status != 'accepting messages':

This comment was marked as spam.

Sign in to view

Comment thread pubsub/google/cloud/pubsub_v1/publisher/future.py Outdated

import queue
import uuid
import time

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

theacodes reviewed Aug 22, 2017
View reviewed changes

theacodes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks mostly good, some small things and nits.

Comment thread pubsub/google/cloud/pubsub_v1/publisher/batch/base.py Outdated
"""
raise NotImplementedError

class Status(object):

This comment was marked as spam.

Sign in to view

Comment thread pubsub/google/cloud/pubsub_v1/publisher/batch/base.py
@@ -0,0 +1,130 @@
# Copyright 2017, Google Inc. All rights reserved.

This comment was marked as spam.

Sign in to view

Comment thread pubsub/google/cloud/pubsub_v1/publisher/batch/base.py
"""
raise NotImplementedError

@property

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread pubsub/google/cloud/pubsub_v1/publisher/batch/thread.py
# commit when the max latency is reached.
self._thread = None
if autocommit and self._settings.max_latency < float('inf'):
self._thread = threading.Thread(target=self.monitor)

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread pubsub/google/cloud/pubsub_v1/publisher/batch/thread.py Outdated
version, which calls this one.
"""
# Update the status.
self._status = 'in-flight'

This comment was marked as spam.

Sign in to view

Comment thread pubsub/google/cloud/pubsub_v1/publisher/client.py
if isinstance(v, six.binary_type):
attrs[k] = v.decode('utf-8')
continue
raise TypeError('All attributes being published to Pub/Sub must '

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread pubsub/google/cloud/pubsub_v1/publisher/exceptions.py Outdated
pass


class TimeoutError(GoogleAPIError):

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread pubsub/google/cloud/pubsub_v1/publisher/futures.py
return False

def running(self):
"""Publishes in Pub/Sub currently may not be canceled.

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread pubsub/google/cloud/pubsub_v1/publisher/futures.py Outdated

# Wait a little while and try again.
time.sleep(_wait)
return self.exception(

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread pubsub/google/cloud/pubsub_v1/types.py Outdated
)


names = ['BatchSettings', 'FlowControl']

This comment was marked as spam.

Sign in to view

lukesneeringer commented Aug 22, 2017

Copy link
Copy Markdown
Contributor Author

@jonparrott I think this is good other than the inner Status class that I asked about.

theacodes commented Aug 22, 2017

Copy link
Copy Markdown
Contributor

@jonparrott I think this is good other than the inner Status class that I asked about.

I answered

lukesneeringer commented Aug 22, 2017

Copy link
Copy Markdown
Contributor Author

Got it. GitHub obscured that and I missed it.

lukesneeringer commented Aug 22, 2017

Copy link
Copy Markdown
Contributor Author

Also, I think this knocks out everything on this PR. Is that accurate?

theacodes commented Aug 22, 2017

Copy link
Copy Markdown
Contributor

Two outstanding items:

  1. https://github.com/GoogleCloudPlatform/google-cloud-python/pull/3463/files#r134590605
  2. https://github.com/GoogleCloudPlatform/google-cloud-python/pull/3463/files#r134535130

lukesneeringer commented Aug 24, 2017

Copy link
Copy Markdown
Contributor Author

Intentionally bypassing CircleCI because:

  • They already ran on pubsub-subscriber, which was branched off of this branch.
  • Same for pubsub-docs.
  • I am setting up a final check to merge pubsub to master, and CI can and will run from that tonight to hopefully merge tomorrow morning.

lukesneeringer merged commit 6bf3359 into pubsub Aug 24, 2017
lukesneeringer deleted the pubsub-publisher branch August 24, 2017 03:49
lukesneeringer mentioned this pull request Aug 24, 2017
Merged
plamut reviewed Dec 22, 2021
View reviewed changes
Comment thread pubsub/google/cloud/pubsub_v1/_gapic.py
Comment on lines +20 to +21
def add_methods(source_class, blacklist=()):
"""Add wrapped versions of the `api` member's methods to the class.

plamut Dec 22, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@lukesneeringer @theacodes @dhermes

Hey folks, happy 2022!

This is a long shot, as the PR is more than 4 years old, but does any of you remember what problem this decorator tried to solve? Why the generated client classes were not directly subclassed by the hand-written classes and instead kept an instance of the generated class in the .api attribute?

The thing is that type checkers issue a lot of complaints about non-existing methods, because they simply cannot know that a lot of these methods are dynamically injected at import time.

We would thus like to subclass the generated classes directly, already have a POC working, but we would like to know if there are any significant downsides to it, if anyone remembers?

Thanks!

parthea pushed a commit that referenced this pull request Oct 21, 2023
chore(deps): update dependency cryptography to v2.9.1 [(#3463)](Googl…
417789f 
…eCloudPlatform/python-docs-samples#3463)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

5 more reviewers
dhermes dhermes left review comments
plamut plamut left review comments
mdietz94 mdietz94 requested changes
tbkneeland tbkneeland requested changes
theacodes theacodes approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

lukesneeringer

Labels

api: pubsub Issues related to the Pub/Sub API. cla: yes This human has signed the Contributor License Agreement. do not merge Indicates a pull request not ready for merge, due to either quality or timing. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

Back | FazBrowse Home | New Git URL