-
Notifications
You must be signed in to change notification settings - Fork 207
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs(samples): Add code sample for optimistic subscribe #1182
Conversation
Here is the summary of changes. You are about to add 1 region tag.
This comment is generated by snippet-bot.
|
0bd7cd1
to
d4bafff
Compare
cc540b6
to
d16bf30
Compare
@kamalaboulhosn Gentle bump. PTAL when free. |
samples/snippets/subscriber.py
Outdated
) | ||
streaming_pull_future.result(timeout=timeout) | ||
except TimeoutError: | ||
print("Subscription already exists. Timed out.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we instead say "Successfully subscribed until the timeout passed."? As it is, it sounds like this is an error state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I've replaced the statement with the suggested statement.
samples/snippets/subscriber.py
Outdated
request={"name": subscription_path, "topic": topic_path} | ||
) | ||
|
||
if subscription: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we do something if this test fails?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the call to create_subscription()
fails, then it would throw an exception and the control would not arrive at the if subscription
check.
python-pubsub/google/pubsub_v1/services/subscriber/client.py
Lines 743 to 744 in 5094605
If the subscription already exists, returns ``ALREADY_EXISTS``. | |
If the corresponding topic doesn't exist, returns ``NOT_FOUND``. |
The check should always evaluate to true and was added as a precautionary null check. I've now additionally included an else clause that raises a ValueError
in case this check fails.
I've now also enclosed the create subscription code in a try / except clause to handle any exceptions thrown by the create_subscription()
call. Reasons for not doing this earlier is to keep the sample code simple, similar to the pubsub_create_pull_subscription()
sample code in the file:
python-pubsub/samples/snippets/subscriber.py
Lines 86 to 94 in 5094605
# Wrap the subscriber in a 'with' block to automatically call close() to | |
# close the underlying gRPC channel when done. | |
with subscriber: | |
subscription = subscriber.create_subscription( | |
request={"name": subscription_path, "topic": topic_path} | |
) | |
print(f"Subscription created: {subscription}") | |
# [END pubsub_create_pull_subscription] |
76e5004
to
acd5472
Compare
aab0fc7
to
103d1d8
Compare
@kamalaboulhosn Addressed comments. PTAL when free. |
Related documentation: https://cloud.google.com/pubsub/docs/troubleshooting#using_excessive_administrative_operations
Fixes #1183 🦕