-
Notifications
You must be signed in to change notification settings - Fork 2.2k
[Feature][Connector-V2] Support multi-table sink feature for TDengine #9215
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
… support_multiSink_tdengine
… support_multiSink_tdengine
… support_multiSink_tdengine
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 PR adds support for multi‐table sink functionality for the TDengine connector, enabling tests to write to multiple tables. Key changes include:
- Removing TDengine from the exclusion list in integration tests.
- Introducing new multi‐table sink test cases and constants in TDengineIT.
- Refactoring configuration and sink writer construction to support multi‐table operations.
Reviewed Changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| seatunnel-e2e/seatunnel-core-e2e/seatunnel-starter-e2e/src/test/java/org/apache/seatunnel/core/starter/seatunnel/SeaTunnelConnectorTest.java | Removed TDengine exclusion to enable TDengine tests. |
| seatunnel-e2e/seatunnel-connector-v2-e2e/connector-tdengine-e2e/src/test/java/org/apache/seatunnel/e2e/connector/tdengine/TDengineIT.java | Added multi-table sink test cases, constants, and updated query methods. |
| seatunnel-connectors-v2/connector-tdengine/src/test/java/org/apache/seatunnel/connectors/seatunnel/tdengine/sink/TDengineSinkWriterTest.java | Refactored sink writer tests to use the new builder-based configuration. |
| seatunnel-connectors-v2/connector-tdengine/src/main/java/org/apache/seatunnel/connectors/seatunnel/tdengine/source/TDengineSourceFactory.java | Updated source factory to support TDengine. |
| seatunnel-connectors-v2/connector-tdengine/src/main/java/org/apache/seatunnel/connectors/seatunnel/tdengine/sink/*.java | Modified sink writer, sink, and sink factory classes to implement multi-table support. |
| seatunnel-connectors-v2/connector-tdengine/src/main/java/org/apache/seatunnel/connectors/seatunnel/tdengine/config/* | Added and updated source and sink configuration and options. |
| seatunnel-ci-tools/src/test/java/org/apache/seatunnel/api/ConnectorOptionCheckTest.java | Updated connector option whitelist by removing TDengine options. |
Files not reviewed (1)
- seatunnel-e2e/seatunnel-connector-v2-e2e/connector-tdengine-e2e/src/test/resources/tdengine/tdengine_fake_to_sink_multitable.conf: Language not supported
Comments suppressed due to low confidence (2)
seatunnel-e2e/seatunnel-connector-v2-e2e/connector-tdengine-e2e/src/test/java/org/apache/seatunnel/e2e/connector/tdengine/TDengineIT.java:62
- [nitpick] Variable name 'testDataCountMulti_Table1' uses underscores which is unconventional in Java non-constant variable naming. Consider renaming it to 'testDataCountMultiTable1' to follow camelCase conventions.
private final int testDataCountMulti_Table1 = 5;
seatunnel-e2e/seatunnel-connector-v2-e2e/connector-tdengine-e2e/src/test/java/org/apache/seatunnel/e2e/connector/tdengine/TDengineIT.java:63
- [nitpick] Variable name 'testDataCountMulti_Table2' uses underscores which is unconventional in Java non-constant variable naming. Consider renaming it to 'testDataCountMultiTable2' to improve readability.
private final int testDataCountMulti_Table2 = 7;
| public static final Option<String> USERNAME = | ||
| Options.key("username") | ||
| .stringType() | ||
| .noDefaultValue() | ||
| .withDescription("The username for TDengine authentication"); | ||
|
|
||
| public static final Option<String> PASSWORD = | ||
| Options.key("password") | ||
| .stringType() | ||
| .noDefaultValue() | ||
| .withDescription("The password for TDengine authentication"); |
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 move all common options in source and sink into abstract common options class.
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.
Done. I've refactored the code to move repeated options into a common options.
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 PR adds support for a multi‐table sink feature for TDengine by updating both the integration tests and the TDengine connector code. Key changes include removing TDengine from the exclusion list in integration tests, adding new multi-write test cases and table creation for additional targets, and refactoring connector configurations and sink writer implementations to support multi‐table writing.
Reviewed Changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| seatunnel-e2e/seatunnel-core-e2e/seatunnel-starter-e2e/src/test/java/org/apache/seatunnel/core/starter/seatunnel/SeaTunnelConnectorTest.java | Removed exclusion for TDengine to include its test runs. |
| seatunnel-e2e/seatunnel-connector-v2-e2e/connector-tdengine-e2e/src/test/java/org/apache/seatunnel/e2e/connector/tdengine/TDengineIT.java | Added new tests and table creation commands for multi-table sink support. |
| seatunnel-connectors-v2/connector-tdengine/* | Updated source and sink factories, configuration classes, and sink writer to support multi-table features. |
| seatunnel-ci-tools/src/test/java/org/apache/seatunnel/api/ConnectorOptionCheckTest.java | Removed TDengine options from the whitelist to align with the updated connector tests. |
Files not reviewed (1)
- seatunnel-e2e/seatunnel-connector-v2-e2e/connector-tdengine-e2e/src/test/resources/tdengine/tdengine_fake_to_sink_multitable.conf: Language not supported
Comments suppressed due to low confidence (2)
seatunnel-e2e/seatunnel-connector-v2-e2e/connector-tdengine-e2e/src/test/java/org/apache/seatunnel/e2e/connector/tdengine/TDengineIT.java:62
- [nitpick] Consider renaming 'testDataCountMulti_Table1' to 'testDataCountMultiTable1' to conform with standard Java camelCase naming conventions.
private final int testDataCountMulti_Table1 = 5;
seatunnel-e2e/seatunnel-connector-v2-e2e/connector-tdengine-e2e/src/test/java/org/apache/seatunnel/e2e/connector/tdengine/TDengineIT.java:63
- [nitpick] Consider renaming 'testDataCountMulti_Table2' to 'testDataCountMultiTable2' to maintain consistency with Java naming conventions.
private final int testDataCountMulti_Table2 = 7;
Purpose of this pull request
[Feature][TDengine] Support multi-table sink feature #5652
Does this PR introduce any user-facing change?
How was this patch tested?
a new e2e test
Check list
New License Guide
release-note.