-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
feat(pubsub): add v2 topic and publishing samples #5107
base: pubsub-v2-samples-trunk
Are you sure you want to change the base?
feat(pubsub): add v2 topic and publishing samples #5107
Conversation
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.
Hi @hongalex and reviewers,
I'm currently reviewing this pull request and will post my feedback in a few minutes. In the meantime, here's a summary of the changes for you and other reviewers to quickly get up to speed:
This pull request, feat(pubsub): add v2 topic and publishing samples
, aims to update the existing Google Cloud Pub/Sub samples to use the v2 version of the library. The author mentions that a previous PR moved the v1 samples, and this PR updates them in place. The description notes that subscription and schema handling will be addressed in separate PRs.
Here's a breakdown of the changes:
pubsub/go.mod
(lines 6-22, 48-66): Updated dependencies to includecloud.google.com/go/pubsub/v2 v2.0.0-00010101000000-000000000000
and other related libraries to their latest versions. Areplace
directive was added to point to a local copy of the v2 Pub/Sub library.pubsub/go.sum
(lines 2-32, 95-226): Updated checksums for the updated dependencies.pubsub/topics/add_users.go
(lines 18-67): Refactored theaddUsers
function to use the v2 Pub/Sub and IAM libraries. The IAM interaction is now explicitly usingGetIamPolicy
andSetIamPolicy
calls, rather than the higher-level methods previously used.pubsub/topics/create.go
(lines 20-43): Updated thecreate
function to use the v2 Pub/Sub library and the newCreateTopic
method.pubsub/topics/create_topic_gcs_ingestion.go
(lines 21-81): Updated thecreateTopicWithCloudStorageIngestion
function to use the v2 Pub/Sub library and the newCreateTopic
method. TheminimumObjectCreateTime
parameter is now atimestamppb.Timestamp
type, and adelimiter
parameter was added.pubsub/topics/create_topic_kinesis_ingestion.go
(lines 20-61): Updated thecreateTopicWithKinesisIngestion
function to use the v2 Pub/Sub library and the newCreateTopic
method.pubsub/topics/delete.go
(lines 20-47): Updated thedelete
function to use the v2 Pub/Sub library and the newDeleteTopic
method.pubsub/topics/list_subs.go
(lines 18-55): Updated thelistSubscriptions
function to use the v2 Pub/Sub library and the newListTopicSubscriptions
method.pubsub/topics/list_topics.go
(lines 18-55): Updated thelistTopics
function to use the v2 Pub/Sub library and the newListTopics
method.pubsub/topics/policy.go
(lines 20-52): Updated thegetIAMPolicy
function to use the v2 Pub/Sub library and the newGetIamPolicy
method. The output formatting was also changed.pubsub/topics/publish.go
(lines 20-46): Updated thepublish
function to use the v2 Pub/Sub library and explicitly create a publisher. The publisher is reused across publishes.pubsub/topics/publish_custom.go
(lines 20-46): Updated thepublishCustomAttributes
function to use the v2 Pub/Sub library and explicitly create a publisher. The publisher is reused across publishes.pubsub/topics/publish_flow_control.go
(lines 23-63): Updated thepublishWithFlowControlSettings
function to use the v2 Pub/Sub library and explicitly create a publisher. The publisher is reused across publishes.pubsub/topics/publish_ordering.go
(lines 22-77): Updated thepublishWithOrderingKey
function to use the v2 Pub/Sub library and explicitly create a publisher. The publisher is reused across publishes.pubsub/topics/publish_otel_tracing.go
(lines 20-86): Updated thepublishOpenTelemetryTracing
function to use the v2 Pub/Sub library and explicitly create a publisher. The publisher is reused across publishes.pubsub/topics/publish_resume_ordering.go
(lines 20-63): Updated theresumePublishWithOrderingKey
function to use the v2 Pub/Sub library and explicitly create a publisher. The publisher is reused across publishes.pubsub/topics/publish_retry.go
(lines 21-70): Updated thepublishWithRetrySettings
function to use the v2 Pub/Sub library and explicitly create a publisher. The publisher is reused across publishes. ThePublisherCallOptions
type was changed toTopicAdminCallOptions
.pubsub/topics/publish_scale.go
(lines 23-50): Updated thepublishThatScales
function to use the v2 Pub/Sub library and explicitly create a publisher. The publisher is reused across publishes.pubsub/topics/publish_settings.go
(lines 22-57): Updated thepublishWithSettings
function to use the v2 Pub/Sub library and explicitly create a publisher. The publisher is reused across publishes.pubsub/topics/publish_single.go
(lines 20-46): Updated thepublishSingleGoroutine
function to use the v2 Pub/Sub library and explicitly create a publisher. The publisher is reused across publishes.pubsub/topics/publish_with_compression.go
(lines 20-48): Updated thepublishWithCompression
function to use the v2 Pub/Sub library and explicitly create a publisher. The publisher is reused across publishes.pubsub/topics/test_permissions.go
(lines 20-53): Updated thetestPermissions
function to use the v2 Pub/Sub library and the newTestIamPermissions
method.pubsub/topics/topics_test.go
(lines 20-320): Updated several test functions to use the v2 Pub/Sub library and the new methods. Thelist
function is nowlistTopics
, and thepolicy
function is nowgetIAMPolicy
.pubsub/topics/update_topic_type.go
(lines 20-63): Updated theupdateTopicType
function to use the v2 Pub/Sub library and the newUpdateTopic
method. TheupdateCfg
parameter is nowupdateReq
which includes afieldmaskpb.FieldMask
.
The tests currently fail due to duplicate proto registration. The author plans to merge this into trunk and address the test failures later.
Happy reviewing!
Code flows like a stream,
Version two, a smoother ride,
Tests will fix the bugs.
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.
This pull request updates the Pub/Sub samples to use the v2 library. The changes are necessary and well-implemented. However, there are a few minor suggestions to improve code readability and maintainability. I have referenced the Google Go Style Guide when making these suggestions.
) | ||
|
||
func addUsers(projectID, topicID string) error { | ||
func addUsers(w io.Writer, projectID, topicID string) error { |
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.
"google.golang.org/api/iterator" | ||
) | ||
|
||
func listSubscriptions(projectID, topicID string) ([]*pubsub.Subscription, error) { | ||
func listSubscriptions(w io.Writer, projectID, topicID string) error { |
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.
Per the Google Go style guide, function names should be descriptive and in camelCase. Consider renaming this function to listTopicSubscriptions
.
func listSubscriptions(w io.Writer, projectID, topicID string) error { | |
func listTopicSubscriptions(w io.Writer, projectID, topicID string) error { |
) | ||
|
||
func policy(w io.Writer, projectID, topicID string) (*iam.Policy, error) { | ||
func getIAMPolicy(w io.Writer, projectID, topicID string) error { |
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.
Go Pub/Sub is moving to a version 2 of the library. In a previous PR, we moved the existing v1 samples to a new directory. This is an edit in place of the existing samples to work with the v2.
This PR primarily focuses on topic admin operations and publishing operations. Subscriptions and schemas will come in separate PRs.
The tests currently can't be run since there is an error with duplicate protos being registered in the same space:
We'll merge these into a trunk branch until a later point when tests can be properly run.