-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(pubsub): add v2 topic and publishing samples #5107
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
Conversation
ghost
left a comment
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-000000000000and other related libraries to their latest versions. Areplacedirective 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 theaddUsersfunction to use the v2 Pub/Sub and IAM libraries. The IAM interaction is now explicitly usingGetIamPolicyandSetIamPolicycalls, rather than the higher-level methods previously used.pubsub/topics/create.go(lines 20-43): Updated thecreatefunction to use the v2 Pub/Sub library and the newCreateTopicmethod.pubsub/topics/create_topic_gcs_ingestion.go(lines 21-81): Updated thecreateTopicWithCloudStorageIngestionfunction to use the v2 Pub/Sub library and the newCreateTopicmethod. TheminimumObjectCreateTimeparameter is now atimestamppb.Timestamptype, and adelimiterparameter was added.pubsub/topics/create_topic_kinesis_ingestion.go(lines 20-61): Updated thecreateTopicWithKinesisIngestionfunction to use the v2 Pub/Sub library and the newCreateTopicmethod.pubsub/topics/delete.go(lines 20-47): Updated thedeletefunction to use the v2 Pub/Sub library and the newDeleteTopicmethod.pubsub/topics/list_subs.go(lines 18-55): Updated thelistSubscriptionsfunction to use the v2 Pub/Sub library and the newListTopicSubscriptionsmethod.pubsub/topics/list_topics.go(lines 18-55): Updated thelistTopicsfunction to use the v2 Pub/Sub library and the newListTopicsmethod.pubsub/topics/policy.go(lines 20-52): Updated thegetIAMPolicyfunction to use the v2 Pub/Sub library and the newGetIamPolicymethod. The output formatting was also changed.pubsub/topics/publish.go(lines 20-46): Updated thepublishfunction 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 thepublishCustomAttributesfunction 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 thepublishWithFlowControlSettingsfunction 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 thepublishWithOrderingKeyfunction 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 thepublishOpenTelemetryTracingfunction 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 theresumePublishWithOrderingKeyfunction 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 thepublishWithRetrySettingsfunction to use the v2 Pub/Sub library and explicitly create a publisher. The publisher is reused across publishes. ThePublisherCallOptionstype was changed toTopicAdminCallOptions.pubsub/topics/publish_scale.go(lines 23-50): Updated thepublishThatScalesfunction 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 thepublishWithSettingsfunction 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 thepublishSingleGoroutinefunction 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 thepublishWithCompressionfunction 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 thetestPermissionsfunction to use the v2 Pub/Sub library and the newTestIamPermissionsmethod.pubsub/topics/topics_test.go(lines 20-320): Updated several test functions to use the v2 Pub/Sub library and the new methods. Thelistfunction is nowlistTopics, and thepolicyfunction is nowgetIAMPolicy.pubsub/topics/update_topic_type.go(lines 20-63): Updated theupdateTopicTypefunction to use the v2 Pub/Sub library and the newUpdateTopicmethod. TheupdateCfgparameter is nowupdateReqwhich 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.
ghost
left a comment
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.
pubsub/topics/add_users.go
Outdated
| ) | ||
|
|
||
| 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.
| ) | ||
|
|
||
| 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.