seatunnel icon indicating copy to clipboard operation
seatunnel copied to clipboard

[Bug][Improve][Connector-v2][ElasticsearchSource] Fix behavior when source empty,Support SourceConfig.SOURCE field empty.

Open CosmosNi opened this issue 11 months ago • 17 comments

[Improve][Connector-v2][ElasticsearchSource] Support SourceConfig.SOURCE field empty.

Purpose of this pull request

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

  • [x] If any new Jar binary package adding in your PR, please add License Notice according New License Guide
  • [x] If necessary, please update the documentation to describe the new feature. https://github.com/apache/seatunnel/tree/dev/docs
  • [x] 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
  • [x] Update the release-note.

CosmosNi avatar Feb 29 '24 09:02 CosmosNi

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

CosmosNi avatar Feb 29 '24 09:02 CosmosNi

I think we should add a e2e test case in https://github.com/apache/seatunnel/blob/045d3e3494eac67cef555c190ecb68638d497349/seatunnel-e2e/seatunnel-connector-v2-e2e/connector-elasticsearch-e2e/src/test/java/org/apache/seatunnel/e2e/connector/elasticsearch/ElasticsearchIT.java#L68

Hisoka-X avatar Mar 06 '24 08:03 Hisoka-X

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

CosmosNi avatar Mar 26 '24 03:03 CosmosNi

Hi @CosmosNi , thanks for add test! Could you retry the test case after merge from dev?

Hisoka-X avatar Mar 26 '24 05:03 Hisoka-X

@Hisoka-X I don't understand. Can you show me how to operate it?

CosmosNi avatar Mar 26 '24 06:03 CosmosNi

image merge code from upstream

Hisoka-X avatar Mar 26 '24 07:03 Hisoka-X

@Hisoka-X already merge code from upstream,PTAL,thanks.

CosmosNi avatar Mar 26 '24 07:03 CosmosNi

The conflict has been resolved,and already add test case ,@iture123 @TyrantLucifer @Hisoka-X PTAL,thanks.

CosmosNi avatar Apr 07 '24 02:04 CosmosNi

I think schema option is excess for ES Source, So, can we comment schema is deprecated in code and document? @EricJoy2048 Yes, in fact, I think source is sufficient for es

CosmosNi avatar Apr 07 '24 14:04 CosmosNi

Thanks @CosmosNi for update, could you follow the suggestion of @EricJoy2048 to mark schema config in ES source as deprecated? Both in doc and code.

Hisoka-X avatar Apr 08 '24 11:04 Hisoka-X

Thanks @CosmosNi for update, could you follow the suggestion of @EricJoy2048 to mark schema config in ES source as deprecated? Both in doc and code.

@Hisoka-X mark schema config in ES source as deprecated already

CosmosNi avatar Apr 08 '24 14:04 CosmosNi

https://github.com/apache/seatunnel/pull/6425/files#diff-8eef53f88ae098e1477f67d8053f946d3267578ec8e68fd472d22c298864f876R60 This line should be deleted. Also please tell user when not config source, the behavior what is.

Hisoka-X avatar Apr 11 '24 03:04 Hisoka-X

@Hisoka-X I have a problem,There are no array types in es,so when source include an array type,The engine will not be able to fight for the job.It is not up to the user to specify the array type to be used by adding parameters

CosmosNi avatar Apr 12 '24 08:04 CosmosNi

@Hisoka-X I have a problem,There are no array types in es,so when source include an array type,The engine will not be able to fight for the job.It is not up to the user to specify the array type to be used by adding parameters

Yes, this is an optional way.

Hisoka-X avatar Apr 12 '24 12:04 Hisoka-X

@Hisoka-X i add a label (array_column) to handle es array type,PTAL,thanks.

CosmosNi avatar Apr 15 '24 03:04 CosmosNi

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

CosmosNi avatar Apr 28 '24 05:04 CosmosNi

@Hisoka-X sorry,I accidentally removed label.

CosmosNi avatar Apr 28 '24 07:04 CosmosNi