-
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 kafka ingestion, azure ingestion samples #5123
feat(pubsub): add kafka ingestion, azure ingestion samples #5123
Conversation
Here is the summary of changes. You are about to add 3 region tags.
This comment is generated by snippet-bot.
|
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 team,
I'm currently reviewing this pull request and will provide feedback shortly. In the meantime, here's a summary for you and other reviewers to quickly get up to speed:
This pull request, authored by hongalex, aims to add Kafka, Azure, and Confluent Cloud ingestion samples to the Pub/Sub client library. The changes include the addition of three new sample files and updates to the test suite.
Summary of Changes:
- New Files:
pubsub/v1samples/topics/create_topic_aws_msk_ingestion.go
: This file adds a sample demonstrating how to create a Pub/Sub topic with AWS MSK ingestion. (Lines 1-61)pubsub/v1samples/topics/create_topic_azure_event_hubs_ingestion.go
: This file adds a sample demonstrating how to create a Pub/Sub topic with Azure Event Hubs ingestion. (Lines 1-67)pubsub/v1samples/topics/create_topic_confluent_cloud_ingestion.go
: This file adds a sample demonstrating how to create a Pub/Sub topic with Confluent Cloud ingestion. (Lines 1-63)
- Modified File:
pubsub/v1samples/topics/topics_test.go
: This file has been updated to include new test cases for the added ingestion samples. (Lines 316-376, 328-387)
Intent:
The intent is to expand the available samples to cover a wider range of ingestion sources, making the library more versatile and useful for users.
And now, a little haiku to lighten the mood:
Code flows like a stream,
New features add to the source,
Reviews make it gleam.
Thanks,
Gemini/code-review-assist
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 adds samples for creating topics with ingestion from Kafka, Azure Event Hubs, and Confluent Cloud. The implementations look correct and efficient. The tests are also well-written and cover the basic scenarios. Here's a summary of the changes made according to the Google Go Style Guide:
- Naming Conventions: Function names are in camelCase and follow Go conventions.
- Error Handling: Errors are handled using
fmt.Errorf
and%w
for wrapping. - Context Usage: Context is used correctly for managing deadlines and cancellations.
- Package Structure: The samples are organized within the
topics
package. - Comments and Documentation: The code is well-commented, and the function comments include start and end markers for documentation generation.
- Testing: Tests are provided for each new function, and they use the
pubsub
emulator for testing against a fake server.
Overall, the code is well-written and easy to understand. Here are a few minor suggestions for improvement.
pubsub/v1samples/topics/create_topic_azure_event_hubs_ingestion.go
Outdated
Show resolved
Hide resolved
…lang-samples into feat-pubsub-ingestion-topics
Adds samples for three new topics: AWS Managed Service for Kafka, Azure Event Hubs, Confluent Cloud managed kafka.