-
Notifications
You must be signed in to change notification settings - Fork 645
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
Google Pub/Sub: Configurable host (for ordering) #2879
Conversation
At least one commit author (bednorz@virtuslab.com) is not linked to a user. See https://help.github.com/en/articles/why-are-my-commits-linked-to-the-wrong-user#commits-are-not-linked-to-any-user |
*/ | ||
def publish(topic: String, | ||
config: PubSubConfig, | ||
overrideHost: java.util.Optional[String], |
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.
Wouldn't String
be enough? If you don't want to pass a host you'd use the other overload?
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.
Right, changed
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.
LGTM.
Thank you! |
If ordering is required on Pub/Sub then messages must be sent only to specific region (using regional Pub/Sub endpoint). If two messages with the same orderng key are sent to two different regions, then ordering is not guaranteed.
All quotes from Pub/Sub documentation.
https://cloud.google.com/pubsub/docs/ordering
https://cloud.google.com/pubsub/docs/resource-location-restriction
It's possible to restrict region on topic level
But:
So the best solution is specify region when message is published.
PR constains proposition with extending publish flow, by new optional parameter with Pub/Sub host.
Example usage:
GooglePubSub.publish(topic, config, Some("europe-west1-pubsub.googleapis.com"));