-
Notifications
You must be signed in to change notification settings - Fork 2.1k
[Fix][Connector-V2] Fix kafka database name #9201
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
Conversation
.../test/java/org/apache/seatunnel/connectors/seatunnel/kafka/source/KafkaSourceConfigTest.java
Outdated
Show resolved
Hide resolved
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.
Pull Request Overview
This pull request fixes an issue with the Kafka connector’s database naming by refactoring the table path construction logic. Key changes include:
- Extraction of the table path resolution into a new method (getTablePathFromSchema) for improved flexibility.
- Updates to the catalog table creation to use the new table path resolution.
- Addition of a new unit test (testDeserializationWithSchema) to validate deserialization behavior when a schema is provided.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| seatunnel-connectors-v2/connector-kafka/src/test/java/org/apache/seatunnel/connectors/seatunnel/kafka/source/KafkaSourceConfigTest.java | Added a unit test to verify deserialization with a provided schema and table identifier. |
| seatunnel-connectors-v2/connector-kafka/src/main/java/org/apache/seatunnel/connectors/seatunnel/kafka/source/KafkaSourceConfig.java | Refactored the table path construction by extracting logic into getTablePathFromSchema to fix database naming issues. |
Comments suppressed due to low confidence (2)
seatunnel-connectors-v2/connector-kafka/src/main/java/org/apache/seatunnel/connectors/seatunnel/kafka/source/KafkaSourceConfig.java:265
- Ensure that the schema configuration key is used consistently across the connector. You might consider aligning the usage of KafkaSourceOptions.SCHEMA and TableSchemaOptions.SCHEMA to prevent potential misconfiguration.
readonlyConfig.getOptional(TableSchemaOptions.SCHEMA)
seatunnel-connectors-v2/connector-kafka/src/test/java/org/apache/seatunnel/connectors/seatunnel/kafka/source/KafkaSourceConfigTest.java:86
- [nitpick] Consider clarifying the test table identifier format (e.g., separating database and table parts) to ensure it aligns with production expectations and avoids ambiguity.
schema.put(TABLE.key(), "test.test");
hailin0
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.
LGTM
waiting for ci passed
Purpose of this pull request
Does this PR introduce any user-facing change?
How was this patch tested?
Check list
New License Guide
release-note.