[Fix] [Kafka Source] kafka source use topic as table name instead of fullName by Cheun99 · Pull Request #8401 · apache/seatunnel

Cheun99 · 2024-12-30T02:14:34Z

Purpose of this pull request

resolve #8396

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

If any new Jar binary package adding in your PR, please add License Notice according
New License Guide
If necessary, please update the documentation to describe the new feature. https://github.com/apache/seatunnel/tree/dev/docs
If you are contributing the connector code, please check that the following files are updated:
1. Update plugin-mapping.properties and add new connector information in it
2. Update the pom file of seatunnel-dist
3. Add ci label in label-scope-conf
4. Add e2e testcase in seatunnel-e2e
5. Update connector plugin_config
Update the release-note.

Hisoka-X

LGTM if ci passes.

Hisoka-X · 2025-01-08T02:25:10Z

+        String topic = readonlyConfig.get(TOPIC).replace(".", "_");
+        TablePath tablePath = TablePath.of("kafka", topic);


Hi why not use TablePath.of("kafka", readonlyConfig.get(TOPIC))?

https://github.com/apache/seatunnel/blob/dev/seatunnel-engine/seatunnel-engine-server/src/main/java/org/apache/seatunnel/engine/server/task/flow/SinkFlowLifeCycle.java#L132

If the kafka topic name contains more than 3 points, when this line of code is executed, it will still be separated by points. If the element is greater than 3, IllegalArgumentException will still be thrown

I think we should fix it. We can save the TablePath directly instead of the corresponding string in SinkFlowLifeCycle.

I tried to save the TablePath before, but it involved other files and many of them were modified

Never mind. Please commit it then we can review it together.

@Hisoka-X Hi，pls have a review

Hisoka-X · 2025-01-09T03:12:10Z

+                    //                    String tableId =
+                    // action.getConfig().getMultipleRowTableId();


Suggested change

// String tableId =

// action.getConfig().getMultipleRowTableId();

Hisoka-X · 2025-01-09T03:12:17Z

 @AllArgsConstructor
 public class SinkConfig implements Config {
-    private String multipleRowTableId;
+    //    private String multipleRowTableId;


Suggested change

// private String multipleRowTableId;

Hisoka-X · 2025-01-09T03:13:08Z

Overall LGTM. Let's waiting to check the ci result.

Hisoka-X

But you lost the change of kafka and test case.

Cheun99 · 2025-01-09T03:20:05Z

But you lost the change of kafka and test case.

ok, I will delete the comment code you just mentioned and add kafka test case

liunaijie

LGTM

Cheun99 · 2025-01-15T01:55:52Z

                        DEFAULT_FIELD_DELIMITER,
                        null);
-        generateTestData(serializer::serializeRow, 0, 100);
+        generateTestData(serializer::serializeRow, 0, 10);


When I run test cases, it always times out. This method took too much time, so I reduced the size of the mock data from 100 to 10

…fullName (apache#8401)

github-actions Bot added connectors-v2 api labels Dec 30, 2024

liunaijie changed the title ~~[Bug] [Kafka Source] Failed when the consume topic name like a.b.c.d~~ [Fix] [Kafka Source] Failed when the consume topic name like a.b.c.d Dec 30, 2024

liunaijie changed the title ~~[Fix] [Kafka Source] Failed when the consume topic name like a.b.c.d~~ [Fix] [Kafka Source] kafka source use topic as table name instead of fullName Dec 30, 2024

Cheun99 marked this pull request as ready for review December 30, 2024 08:04

Hisoka-X previously approved these changes Jan 2, 2025

View reviewed changes

github-actions Bot added approved reviewed labels Jan 2, 2025

Cheun99 dismissed Hisoka-X’s stale review via 2ec6316 January 3, 2025 05:52

github-actions Bot added core SeaTunnel core module Zeta and removed approved reviewed labels Jan 3, 2025

Cheun99 force-pushed the fix_kafka_topic branch from eab9490 to 29a2ddb Compare January 6, 2025 02:38

github-actions Bot removed core SeaTunnel core module Zeta connectors-v2 labels Jan 6, 2025

Cheun99 force-pushed the fix_kafka_topic branch from 29a2ddb to 428e2f6 Compare January 7, 2025 08:09

github-actions Bot added connectors-v2 e2e and removed api labels Jan 7, 2025

Cheun99 force-pushed the fix_kafka_topic branch from c88c3ad to 643f82c Compare January 7, 2025 14:28

Hisoka-X reviewed Jan 8, 2025

View reviewed changes

Cheun99 force-pushed the fix_kafka_topic branch from 61686ed to 25f720b Compare January 8, 2025 11:03

Cheun99 mentioned this pull request Jan 9, 2025

[Fix][connector-kafka] fix kafka topic contain dot #8479

Closed

4 tasks

Cheun99 force-pushed the fix_kafka_topic branch from 25f720b to 8754e65 Compare January 9, 2025 02:50

github-actions Bot removed the connectors-v2 label Jan 9, 2025

github-actions Bot added core SeaTunnel core module Zeta api and removed e2e kafka labels Jan 9, 2025

Hisoka-X reviewed Jan 9, 2025

View reviewed changes

github-actions Bot added e2e connectors-v2 labels Jan 9, 2025

Cheun99 force-pushed the fix_kafka_topic branch from 0d3b5fd to 38e5925 Compare January 11, 2025 05:15

Cheun99 added 4 commits January 14, 2025 10:27

update table identifier to path

758efe7

remove comment code and add kafka test

7889e7e

update kafka config

afd7259

reduce data size to resolve timeout

b0f4bc9

Cheun99 force-pushed the fix_kafka_topic branch from 38e5925 to b0f4bc9 Compare January 14, 2025 02:29

liunaijie approved these changes Jan 15, 2025

View reviewed changes

github-actions Bot added approved reviewed labels Jan 15, 2025

Cheun99 commented Jan 15, 2025

View reviewed changes

Hisoka-X approved these changes Jan 15, 2025

View reviewed changes

Hisoka-X merged commit 3d4f4bb into apache:dev Jan 15, 2025

litiliu pushed a commit to litiliu/seatunnel that referenced this pull request Jan 16, 2025

[Fix] [Kafka Source] kafka source use topic as table name instead of …

11a25c6

…fullName (apache#8401)

liunaijie mentioned this pull request Mar 1, 2025

[fix][kafka] fix kafka topic with multiple dot #8875

Closed

4 tasks

		String topic = readonlyConfig.get(TOPIC).replace(".", "_");
		TablePath tablePath = TablePath.of("kafka", topic);

		// String tableId =
		// action.getConfig().getMultipleRowTableId();

Uh oh!

Conversation

Cheun99 commented Dec 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose of this pull request

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

Uh oh!

Hisoka-X left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hisoka-X commented Jan 9, 2025

Uh oh!

Hisoka-X left a comment

Choose a reason for hiding this comment

Uh oh!

Cheun99 commented Jan 9, 2025

Uh oh!

liunaijie left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Cheun99 commented Dec 30, 2024 •

edited

Loading