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

[Improve][Connector][PulsarSource]Improve pulsar deserialization #3990

Closed

lightzhao wants to merge 22 commits into apache:dev from lightzhao:pulsar-source-desrialize

Contributor

lightzhao commented Jan 23, 2023

Purpose of this pull request

Improve pulsar deserialization
1.Add format and field_delimiter configuration.
2.Rich deserialization methods.

Check list

Code changed are covered with tests, or it does not need tests for reason:
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/incubator-seatunnel/tree/dev/docs
If you are contributing the connector code, please check that the following files are updated:
1. Update change log that in connector document. For more details you can refer to connector-v2
2. Update plugin-mapping.properties and add new connector information in it
3. Update the pom file of seatunnel-dist
Update the release-note.


          Improve pulsar deserialization

2ca45d3

Hisoka-X added improve connectors-v2 labels

Hisoka-X reviewed

View reviewed changes

...lsar/src/main/java/org/apache/seatunnel/connectors/seatunnel/pulsar/source/PulsarSource.java Outdated

+                                      .build();
+                          } else {
+                              // TODO: use format SPI
+                              throw new SeaTunnelJsonFormatException(CommonErrorCode.UNSUPPORTED_DATA_TYPE,

Member

Hisoka-X Jan 24, 2023

Can't throw UNSUPPORTED_DATA_TYPE, CommonErrorCode.UNSUPPORTED_OPERATION maybe better.

Contributor Author

lightzhao Jan 24, 2023

done.

Member

Hisoka-X commented Jan 24, 2023 •

edited

Loading

Please add change to change log in pulsar doc


          Improve pulsar deserialization

5de7b23

Contributor Author

lightzhao commented Jan 24, 2023

Please add change to change log in pulsar doc

done.

Hisoka-X reviewed

View reviewed changes

docs/en/connector-v2/source/pulsar.md Show resolved Hide resolved


          Update docs/en/connector-v2/source/pulsar.md

f6e0f2d

Member

Hisoka-X commented Jan 24, 2023

Please add schema / format config into e2e test case.

lightzhao added 2 commits

February 16, 2023 18:53


          add pulsar connector e2e

a474475


          Merge branch 'pulsar-source-desrialize' of github.com:lightzhao/incub…

19e2126

…ator-seatunnel into pulsar-source-desrialize

Contributor Author

lightzhao commented Feb 16, 2023

Please add schema / format config into e2e test case.

@Hisoka-X add complete.


          Merge branch 'dev' into pulsar-source-desrialize

aa8b6d0

lightzhao requested a review from Hisoka-X

February 16, 2023 11:05

Member

Hisoka-X commented Feb 16, 2023

Hi, please fix code style.

lightzhao added 4 commits

February 17, 2023 10:26


          fix code style.

fc3248f


          Merge branch 'pulsar-source-desrialize' of github.com:lightzhao/incub…

dbd2de8

…ator-seatunnel into pulsar-source-desrialize


          fix code style.

c330c00


          fix code style.

b4824dc

Contributor Author

lightzhao commented Feb 17, 2023

Hi, please fix code style.

done.

Hisoka-X requested a review from ashulin

February 18, 2023 02:16

Contributor Author

lightzhao commented Feb 22, 2023

@Hisoka-X @ashulin @TyrantLucifer PTAL.

Contributor Author

lightzhao commented Feb 27, 2023

@EricJoy2048 @Hisoka-X @ashulin @TyrantLucifer PTAL.

TyrantLucifer requested changes

View reviewed changes

Member

TyrantLucifer left a comment

Please fix the ci error.

Contributor Author

lightzhao commented Feb 28, 2023

Please fix the ci error.
I don't know why these two ci errors occur occasionally. My local compilation is all right.

Contributor Author

lightzhao commented Mar 3, 2023 •

edited

Loading

lightzhao requested review from TyrantLucifer and removed request for Hisoka-X and ashulin

March 17, 2023 01:23

Contributor Author

lightzhao commented Mar 30, 2023

Hello everyone, who has time to help review this pr. thanks.

lightzhao requested review from Hisoka-X and removed request for ashulin

March 30, 2023 03:09

Contributor Author

lightzhao commented Mar 31, 2023

@TyrantLucifer @Hisoka-X @ashulin PTAL , thanks.

lightzhao requested a review from TyrantLucifer

April 3, 2023 02:40

Contributor Author

lightzhao commented Apr 3, 2023

Hello everyone, who has time to help review this pr. thanks. @EricJoy2048 @TyrantLucifer @Hisoka-X @ashulin

lightzhao requested a review from ashulin

April 4, 2023 02:12

TyrantLucifer reviewed

View reviewed changes

seatunnel-e2e/seatunnel-connector-v2-e2e/connector-pulsar-e2e/pom.xml Outdated

+                          <scope>test</scope>
+                      </dependency>
+                      <dependency>
+                          <groupId>org.testcontainers</groupId>

Member

TyrantLucifer Apr 4, 2023

Why add this?

Contributor Author

lightzhao Apr 4, 2023

deleted.

seatunnel-e2e/seatunnel-connector-v2-e2e/connector-pulsar-e2e/pom.xml Outdated

Comment on lines 89 to 93

+                      <dependency>
+                          <groupId>junit</groupId>
+                          <artifactId>junit</artifactId>
+                          <version>4.12</version>
+                          <scope>compile</scope>

Member

TyrantLucifer Apr 4, 2023

Why add this?

Contributor Author

lightzhao Apr 4, 2023

deleted.

seatunnel-e2e/seatunnel-connector-v2-e2e/connector-pulsar-e2e/pom.xml Outdated

+                      <dependency>
+                          <groupId>org.testcontainers</groupId>
+                          <artifactId>pulsar</artifactId>
+                          <version>1.17.6</version>

Member

TyrantLucifer Apr 4, 2023

Use properties

Contributor Author

lightzhao Apr 4, 2023

done.

seatunnel-e2e/seatunnel-connector-v2-e2e/connector-pulsar-e2e/pom.xml Outdated

+                      <dependency>
+                          <groupId>org.testcontainers</groupId>
+                          <artifactId>pulsar</artifactId>
+                          <version>1.17.6</version>

Member

TyrantLucifer Apr 4, 2023

Use properties

Contributor Author

lightzhao Apr 4, 2023

done.

seatunnel-e2e/seatunnel-connector-v2-e2e/connector-pulsar-e2e/pom.xml

+                      <dependency>
+                          <groupId>org.slf4j</groupId>
+                          <artifactId>jul-to-slf4j</artifactId>
+                          <version>${slf4j.version}</version>

Member

TyrantLucifer Apr 4, 2023

Why add this?

Contributor Author

lightzhao Apr 4, 2023

This class is needed when the test is running, and this log class is missing when ci.

Member

TyrantLucifer Apr 6, 2023

Contributor Author

lightzhao Apr 7, 2023

Contributor Author

lightzhao Apr 10, 2023

@hailin0 PTAL. thanks.
This pr time is quite long, there are other pr waiting for it.

Member

hailin0 May 19, 2023

LGTM


          update pom.

ce42c75

lightzhao requested a review from TyrantLucifer

April 4, 2023 06:08

lightzhao mentioned this pull request

[OnlineMeeting&April.4]SeaTunnel community meeting Topic collect meeting #4496

Closed

3 tasks

Contributor Author

lightzhao commented Apr 6, 2023

@EricJoy2048 @TyrantLucifer @Hisoka-X @ashulin Please approve CI check,thanks.

Contributor Author

lightzhao commented Apr 6, 2023

Check did not find a failure, why is it prompted 'Some checks were not successful', can you retry the check.

Member

Hisoka-X commented Apr 6, 2023

Check did not find a failure, why is it prompted 'Some checks were not successful', can you retry the check.

Some check timout and be canceled. Never mind, ci already pass.

Contributor Author

lightzhao commented Apr 12, 2023

@EricJoy2048 @TyrantLucifer @Hisoka-X @ashulin PTAL,thanks.

Contributor Author

lightzhao commented Apr 13, 2023

hi，all，Who has time to help review this pr, it takes a long time, I can add e2e test to #4382 PulsarSink after completion.

Contributor Author

lightzhao commented Apr 17, 2023

@EricJoy2048 @TyrantLucifer @Hisoka-X @ashulin PTAL, thanks.


          Merge branch 'dev' into pulsar-source-desrialize

ac53cae

Contributor Author

lightzhao commented Apr 18, 2023

Is there a problem with e2e?


          Merge branch 'dev' into pulsar-source-desrialize

7dd1844

Contributor Author

lightzhao commented May 4, 2023

please approve ci. thanks.

Contributor Author

lightzhao commented May 6, 2023

@Hisoka-X @TyrantLucifer @hailin0 please approve ci. thanks.

Member

Hisoka-X commented May 6, 2023

@Hisoka-X @TyrantLucifer @hailin0 please approve ci. thanks.

Sorry for late response, done

Contributor Author

lightzhao commented May 6, 2023

@Hisoka-X @TyrantLucifer @ashulin @hailin0 @EricJoy2048 PTAL, thanks.

Contributor Author

lightzhao commented May 6, 2023

Please ensure that your pull request's CI/CD process passes. Thank you.

done.

Contributor Author

lightzhao commented May 8, 2023

@Hisoka-X @TyrantLucifer @ashulin @hailin0 @EricJoy2048 PTAL, thanks.

Contributor Author

lightzhao commented May 9, 2023

Hi all ,Who has time to review , it's been a long time, #4382 pulsar sink is waiting for this pr.

EricJoy2048 reviewed

View reviewed changes

.../src/main/java/org/apache/seatunnel/connectors/seatunnel/pulsar/config/SourceProperties.java

+                  public static final Option<String> FORMAT =
+                          Options.key("format")
+                                  .stringType()
+                                  .noDefaultValue()

Member

EricJoy2048 May 10, 2023

No defaule value? Or the default value is json? I suggest define the default value in option. public static final String DEFAULT_FORMAT = "json"; is not a good way.

Member

hailin0 May 19, 2023

Contributor Author

lightzhao May 24, 2023

I've been a little busy recently, I'll update it in the next few days.

.../src/main/java/org/apache/seatunnel/connectors/seatunnel/pulsar/config/SourceProperties.java

+                  public static final Option<String> FIELD_DELIMITER =
+                          Options.key("field_delimiter")
+                                  .stringType()
+                                  .noDefaultValue()

Member

EricJoy2048 May 10, 2023

same as above.

...l-connector-v2-e2e/connector-pulsar-e2e/src/test/resources/pulsarsource_text_to_console.conf

+              }
+              sink {
+                Console {}

Member

EricJoy2048 May 10, 2023

I think you need check the data read from the source. You can add a LocalFile Sink Or Pulsar Sink and then read the data from the target file(or topic when you use Pulsar Sink). Check every row and column.

Contributor Author

lightzhao commented May 24, 2023

#4111 This ability has been merged, I will close this pr.

lightzhao closed this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

EricJoy2048 EricJoy2048 left review comments

hailin0 hailin0 left review comments

Hisoka-X Awaiting requested review from Hisoka-X

ashulin Awaiting requested review from ashulin

TyrantLucifer Awaiting requested review from TyrantLucifer

Labels

connectors-v2 improve