You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@seatunnel.apache.org by "kpretty (via GitHub)" <gi...@apache.org> on 2023/03/26 10:05:20 UTC
[GitHub] [incubator-seatunnel] kpretty opened a new pull request, #4233: [Fix][Connector-V2][ES]source deserializer error and inappropriate
kpretty opened a new pull request, #4233:
URL: https://github.com/apache/incubator-seatunnel/pull/4233
## Purpose of this pull request
### for timestamp type data this is wrong
<img width="612" alt="时间解析源代码" src="https://user-images.githubusercontent.com/77819741/221504078-f4d5ba85-baa5-442f-9e90-da4b579f5d29.png">
The error message is as follows
<img width="1185" alt="时间戳转换失败" src="https://user-images.githubusercontent.com/77819741/221504328-0f9254c3-f5a7-4efd-9890-e76bf94986e4.png">
Therefore, for the timestamp type, it should be parsed into LocalDateTime like this
```java
LocalDateTime.ofInstant(Instant.ofEpochMilli(Long.parseLong(fieldValue)), ZoneId.systemDefault());
```
### string parse inappropriate
It is inappropriate to directly call `toString()` for string type data, and the sink will receive redundant `"`
<img width="1167" alt="处理 TextNode 不恰当" src="https://user-images.githubusercontent.com/77819741/221505061-10d88900-6252-40c6-a08a-87fa192939c5.png">
### final
The current pr has already dealt with the above problems, and the final effect is as follows
<img width="1325" alt="正确处理" src="https://user-images.githubusercontent.com/77819741/221505203-f3a6fe4a-f4e6-48f3-b2e0-5c0af99377a7.png">
## Check list
* [x] 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](https://github.com/apache/incubator-seatunnel/blob/dev/docs/en/contribution/new-license.md)
* [ ] 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](https://github.com/apache/incubator-seatunnel/tree/dev/docs/en/connector-v2)
2. Update [plugin-mapping.properties](https://github.com/apache/incubator-seatunnel/blob/dev/plugin-mapping.properties) and add new connector information in it
3. Update the pom file of [seatunnel-dist](https://github.com/apache/incubator-seatunnel/blob/dev/seatunnel-dist/pom.xml)
* [ ] Update the [`release-note`](https://github.com/apache/incubator-seatunnel/blob/dev/release-note.md).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] Hisoka-X commented on pull request #4233: [Fix][Connector-V2][ES]source deserializer error and inappropriate
Posted by "Hisoka-X (via GitHub)" <gi...@apache.org>.
Hisoka-X commented on PR #4233:
URL: https://github.com/apache/incubator-seatunnel/pull/4233#issuecomment-1454736865
Can you fix e2e for this? Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] kpretty commented on pull request #4233: [Fix][Connector-V2][ES]source deserializer error and inappropriate
Posted by "kpretty (via GitHub)" <gi...@apache.org>.
kpretty commented on PR #4233:
URL: https://github.com/apache/incubator-seatunnel/pull/4233#issuecomment-1473064416
@EricJoy2048 fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] TyrantLucifer commented on pull request #4233: [Fix][Connector-V2][ES]source deserializer error and inappropriate
Posted by "TyrantLucifer (via GitHub)" <gi...@apache.org>.
TyrantLucifer commented on PR #4233:
URL: https://github.com/apache/incubator-seatunnel/pull/4233#issuecomment-1446495555
> e2e for es seems to be problematic
Yup, it seems that this change effect the origin test cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] kpretty commented on pull request #4233: [Fix][Connector-V2][ES]source deserializer error and inappropriate
Posted by "kpretty (via GitHub)" <gi...@apache.org>.
kpretty commented on PR #4233:
URL: https://github.com/apache/incubator-seatunnel/pull/4233#issuecomment-1454963246
@Hisoka-X @TyrantLucifer TBR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] kpretty commented on pull request #4233: [Fix][Connector-V2][ES]source deserializer error and inappropriate
Posted by "kpretty (via GitHub)" <gi...@apache.org>.
kpretty commented on PR #4233:
URL: https://github.com/apache/incubator-seatunnel/pull/4233#issuecomment-1467258054
@TyrantLucifer @Hisoka-X This conflict is caused by the different ways of processing data types that es is not mapped to. The processing logic of the dev branch throws an exception, but I think this method is not very friendly. Is it better to give a default type user experience
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] Hisoka-X commented on pull request #4233: [Fix][Connector-V2][ES]source deserializer error and inappropriate
Posted by "Hisoka-X (via GitHub)" <gi...@apache.org>.
Hisoka-X commented on PR #4233:
URL: https://github.com/apache/incubator-seatunnel/pull/4233#issuecomment-1454748790
> @Hisoka-X ok, I'll fix it in the next pr
ok for me, but we will waiting all ci pass to merge this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] kpretty closed pull request #4233: [Fix][Connector-V2][ES]source deserializer error and inappropriate
Posted by "kpretty (via GitHub)" <gi...@apache.org>.
kpretty closed pull request #4233: [Fix][Connector-V2][ES]source deserializer error and inappropriate
URL: https://github.com/apache/incubator-seatunnel/pull/4233
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] kpretty commented on pull request #4233: [Fix][Connector-V2][ES]source deserializer error and inappropriate
Posted by "kpretty (via GitHub)" <gi...@apache.org>.
kpretty commented on PR #4233:
URL: https://github.com/apache/incubator-seatunnel/pull/4233#issuecomment-1484066225
@iture123 there is indeed a problem, I have moved the logic of timestamp processing into `parseDate`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] iture123 commented on a diff in pull request #4233: [Fix][Connector-V2][ES]source deserializer error and inappropriate
Posted by "iture123 (via GitHub)" <gi...@apache.org>.
iture123 commented on code in PR #4233:
URL: https://github.com/apache/incubator-seatunnel/pull/4233#discussion_r1140900039
##########
seatunnel-connectors-v2/connector-elasticsearch/src/main/java/org/apache/seatunnel/connectors/seatunnel/elasticsearch/serialize/source/DefaultSeaTunnelRowDeserializer.java:
##########
@@ -150,7 +158,8 @@ Object convertValue(SeaTunnelDataType<?> fieldType, String fieldValue)
LocalDateTime localDateTime = parseDate(fieldValue);
return localDateTime.toLocalTime();
} else if (LocalTimeType.LOCAL_DATE_TIME_TYPE.equals(fieldType)) {
Review Comment:
ES support Multiple date formats, and you should enhance it without removing the old implementation.ES date type can be defined as follows
{
"type": "date",
"format": "yyyy-MM-dd HH:mm:ss||yyyy-MM-dd||epoch_millis"
}
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] EricJoy2048 commented on pull request #4233: [Fix][Connector-V2][ES]source deserializer error and inappropriate
Posted by "EricJoy2048 (via GitHub)" <gi...@apache.org>.
EricJoy2048 commented on PR #4233:
URL: https://github.com/apache/incubator-seatunnel/pull/4233#issuecomment-1473011446
Please fix the conflicts
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] kpretty commented on pull request #4233: [Fix][Connector-V2][ES]source deserializer error and inappropriate
Posted by "kpretty (via GitHub)" <gi...@apache.org>.
kpretty commented on PR #4233:
URL: https://github.com/apache/incubator-seatunnel/pull/4233#issuecomment-1445890427
e2e for es seems to be problematic
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] TyrantLucifer commented on pull request #4233: [Fix][Connector-V2][ES]source deserializer error and inappropriate
Posted by "TyrantLucifer (via GitHub)" <gi...@apache.org>.
TyrantLucifer commented on PR #4233:
URL: https://github.com/apache/incubator-seatunnel/pull/4233#issuecomment-1454801547
@iture123 PTAL.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] kpretty commented on pull request #4233: [Fix][Connector-V2][ES]source deserializer error and inappropriate
Posted by "kpretty (via GitHub)" <gi...@apache.org>.
kpretty commented on PR #4233:
URL: https://github.com/apache/incubator-seatunnel/pull/4233#issuecomment-1454816515
> > > @Hisoka-X ok, I'll fix it in the next pr
> >
> >
> > ok for me, but we will waiting all ci pass to merge this PR.
>
> CI also has some problems:
>
> 
Yes, I'm testing locally
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] TyrantLucifer commented on pull request #4233: [Fix][Connector-V2][ES]source deserializer error and inappropriate
Posted by "TyrantLucifer (via GitHub)" <gi...@apache.org>.
TyrantLucifer commented on PR #4233:
URL: https://github.com/apache/incubator-seatunnel/pull/4233#issuecomment-1467238840
cc @iture123 PTAL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] TyrantLucifer commented on pull request #4233: [Fix][Connector-V2][ES]source deserializer error and inappropriate
Posted by "TyrantLucifer (via GitHub)" <gi...@apache.org>.
TyrantLucifer commented on PR #4233:
URL: https://github.com/apache/incubator-seatunnel/pull/4233#issuecomment-1454799694
> > @Hisoka-X ok, I'll fix it in the next pr
>
> ok for me, but we will waiting all ci pass to merge this PR.
CI also has some problems
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] kpretty commented on pull request #4233: [Fix][Connector-V2][ES]source deserializer error and inappropriate
Posted by "kpretty (via GitHub)" <gi...@apache.org>.
kpretty commented on PR #4233:
URL: https://github.com/apache/incubator-seatunnel/pull/4233#issuecomment-1454743281
@Hisoka-X ok, I'll fix it in the next pr
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] TyrantLucifer merged pull request #4233: [Fix][Connector-V2][ES]source deserializer error and inappropriate
Posted by "TyrantLucifer (via GitHub)" <gi...@apache.org>.
TyrantLucifer merged PR #4233:
URL: https://github.com/apache/incubator-seatunnel/pull/4233
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org