-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat: Add MKC Code Examples #5359
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
base: main
Are you sure you want to change the base?
Conversation
connect clusters and connectors * Managed Kafka Connect Go Code Samples * Adds fake client for testing Connect clusters
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.
Summary of Changes
Hello @salmany, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces comprehensive Go code examples for Google Cloud's Managed Kafka Connect (MKC) service, covering both cluster and connector management. It also includes a new fake client to facilitate robust testing of these functionalities.
Highlights
- Managed Kafka Connect Go Samples: I've added new Go code examples demonstrating how to interact with Managed Kafka Connect clusters and various types of connectors (MirrorMaker, Pub/Sub Source/Sink, GCS Sink, BigQuery Sink).
- Enhanced Fake Client for Testing: I've extended the internal/managedkafka/fake/fake.go with a fakeManagedKafkaConnectServer to simulate MKC API calls, enabling efficient and isolated testing of the new samples. This includes mock implementations for all CRUD and lifecycle operations for clusters and connectors.
- Comprehensive Test Coverage: I've included new test files (connect_clusters_test.go and connectors_test.go) that utilize the fake client to ensure the correctness and reliability of the new Managed Kafka Connect examples.
- Go Module Dependency Updates: I've updated the managedkafka/go.mod file to align with the latest versions of cloud.google.com/go/managedkafka and other Google Cloud Go client libraries, ensuring compatibility and access to the newest features.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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.
Code Review
This pull request introduces new code examples for Managed Kafka Connect (MKC) clusters and connectors, along with a fake client implementation for testing purposes. The new examples are well-structured and follow the existing patterns in the repository. My review identified a recurring issue in the new test files where a shared buffer is not being reset between sub-tests. This could lead to unreliable test outcomes. I've provided specific suggestions to address this to improve test isolation and reliability.
// projectID := "my-project-id" | ||
// region := "us-central1" | ||
// clusterID := "my-connect-cluster" | ||
// memoryBytes := 13958643712 // 13 GiB in bytes |
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.
It's better to change the value of memory to the multiple of the CPU, like 24 GiB (2 GiB*12 vCPU)
MemoryBytes: 12884901888, // 12 GiB in bytes | ||
} | ||
|
||
connectCluster := &managedkafkapb.ConnectCluster{ |
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 also add the NetworkConfig here?
Maybe also add a comment about extra subnets and DNS domains.
// Cloud Storage Sink sample connector configuration | ||
config := map[string]string{ | ||
"connector.class": "io.aiven.kafka.connect.gcs.GcsSinkConnector", | ||
"tasks.max": "1", |
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.
tasks.max=3
Please also change it for other connectors
managedkafka "cloud.google.com/go/managedkafka/apiv1" | ||
) | ||
|
||
func createMirrorMakerConnector(w io.Writer, projectID, region, connectClusterID, connectorID, sourceDNS, targetDNS, topicName string, opts ...option.ClientOption) 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.
- Please remove security related configs.
- offsetSyncsReplicationFactor is not a default param on the UI
- tasks.max=3 is a default param on the UI.
- It's worth mentioning that the target cluster is usually the primary cluster
- Please change the name to
CreateMirrorMaker2SourceConnector
. MirrorMaker2 is the new version of MirrorMaker. So we'd better use MirrorMaker2 here.
Adds code examples for Managed Kafka Connect (MKC) clusters and connectors
Contains code samples for the following:
1. Mirrormaker 2.0
2. Pub/Sub Source
3. Pub/Sub Sink
4. Cloud Storage Sink
5. BigQuery Sink
Description
Fixes https://buganizer.corp.google.com/issues/430087669
Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google.
Checklist
go test -v ./..
(see Testing)gofmt
(see Formatting)go vet
(see Formatting)