Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Docs] fix kafka format doc #6633

Merged
merged 1 commit into from
Apr 7, 2024
Merged

[Docs] fix kafka format doc #6633

merged 1 commit into from
Apr 7, 2024

Conversation

liunaijie
Copy link
Member

Purpose of this pull request

close #6632

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

@liunaijie
Copy link
Member Author

I noticed another thing:
KafkaSinkFactory has format option check, even the debezium_json not in expect values but still can pass the check
image

litiliu
litiliu reviewed Apr 2, 2024
View reviewed changes
docs/en/connector-v2/sink/Kafka.md
| format | String | No | json | Data format. The default format is json. Optional text format, canal-json, debezium-json and avro.If you use json or text format. The default field separator is ", ". If you customize the delimiter, add the "field_delimiter" option.If you use canal format, please refer to [canal-json](../formats/canal-json.md) for details.If you use debezium format, please refer to [debezium-json](../formats/debezium-json.md) for details. |
| field_delimiter | String | No | , | Customize the field delimiter for data format. |
| common-options | | No | - | Source plugin common parameters, please refer to [Source Common Options](common-options.md) for details |
| Name | Type | Required | Default | Description |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you help explain that changing those lines in the doc helps to solve the #6632?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in https://github.com/apache/seatunnel/blob/dev/seatunnel-connectors-v2/connector-kafka/src/main/java/org/apache/seatunnel/connectors/seatunnel/kafka/config/MessageFormat.java enum, it's debezium_json not debezium-json. so throw debezium-json is not supported error

litiliu
litiliu reviewed Apr 2, 2024
View reviewed changes
Copy link
Contributor

@litiliu litiliu left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems the canal-json is not an incorrect word, why do we need to change it?

@liunaijie liunaijie changed the title [Docs] fix kafka format typo [Docs] fix kafka format doc Apr 3, 2024
@liunaijie
Copy link
Member Author

Seems the canal-json is not an incorrect word, why do we need to change it?

https://github.com/apache/seatunnel/blob/dev/seatunnel-connectors-v2/connector-kafka/src/main/java/org/apache/seatunnel/connectors/seatunnel/kafka/config/MessageFormat.java#L23 there has canal_json format

@litiliu
Copy link
Contributor

litiliu commented Apr 3, 2024 •
edited
Loading

Seems the canal-json is not an incorrect word, why do we need to change it?

https://github.com/apache/seatunnel/blob/dev/seatunnel-connectors-v2/connector-kafka/src/main/java/org/apache/seatunnel/connectors/seatunnel/kafka/config/MessageFormat.java#L23 there has canal_json format

is it better to edit the Enum class MessageFormat to keep the original names, while setting the value to canal-json, and compare the value later? like this, CANAL_JSON("canal-json")

@liunaijie
Copy link
Member Author

Seems the canal-json is not an incorrect word, why do we need to change it?

https://github.com/apache/seatunnel/blob/dev/seatunnel-connectors-v2/connector-kafka/src/main/java/org/apache/seatunnel/connectors/seatunnel/kafka/config/MessageFormat.java#L23 there has canal_json format

is it better to edit the Enum class MessageFormat to keep the original names, while setting the value to canal-json, and compare the value later? like this, CANAL_JSON("canal-json")

this is the document issue, you can see the demo config from the canal-json document and debezium-json document. they are
debezium_json and canal_json instead of debezium-json.

https://github.com/apache/seatunnel/blob/dev/docs/en/connector-v2/formats/debezium-json.md?plain=1#L91
https://github.com/apache/seatunnel/blob/dev/docs/en/connector-v2/formats/canal-json.md?plain=1#L98

litiliu
litiliu approved these changes Apr 3, 2024
View reviewed changes
Copy link
Contributor

@litiliu litiliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@hailin0 hailin0 merged commit f1601e3 into apache:dev Apr 7, 2024
11 checks passed
@liunaijie liunaijie deleted the typo branch April 10, 2024 02:27
chaorongzhi pushed a commit to chaorongzhi/seatunnel that referenced this pull request Aug 21, 2024
@liunaijie
[Docs] fix kafka format typo (apache#6633)
167a866
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@litiliu litiliu litiliu approved these changes

@EricJoy2048 EricJoy2048 EricJoy2048 approved these changes

@hailin0 hailin0 hailin0 approved these changes

Assignees
No one assigned
Labels
approved reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug] [Kafka] Kafka format debezium-json is not supported
4 participants
@liunaijie @litiliu @hailin0 @EricJoy2048