[Hotfix] fix http source can not read yyyy-MM-dd HH:mm:ss format bug & Improve DateTime Utils #6601

EricJoy2048 · 2024-03-28T04:05:38Z

Purpose of this pull request

Fix http source will dead cycle when url is error.
Fix http source can not read datetime format yyyy-MM-dd HH:mm:ss.
Fix txt read can not read different column with different datetime format bug.

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 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.

liunaijie · 2024-04-03T02:43:58Z

seatunnel-common/src/main/java/org/apache/seatunnel/common/utils/DateTimeUtils.java

@@ -101,4 +274,66 @@ public static Formatter parse(String format) {
            throw new IllegalArgumentException(errorMsg);
        }
    }
+
+    public static void main(String[] args) {


this test can move to UT

Sorry, I will do it.

TyrantLucifer

Please add ut/it for DateTimeUtils and DateUtils

TyrantLucifer · 2024-04-03T03:10:55Z

seatunnel-common/src/main/java/org/apache/seatunnel/common/utils/DateTimeUtils.java

@@ -101,4 +274,66 @@ public static Formatter parse(String format) {
            throw new IllegalArgumentException(errorMsg);
        }
    }
+
+    public static void main(String[] args) {


TyrantLucifer · 2024-04-03T03:20:55Z

...el-format-text/src/main/java/org/apache/seatunnel/format/text/TextDeserializationSchema.java

            case TIME:
-                return TimeUtils.parse(field, timeFormatter);
+                TemporalAccessor parsedTime = TIME_FORMAT.parse(field);
+                return parsedTime.query(TemporalQueries.localTime());
            case TIMESTAMP:


Why change this logic? What are the benefits?

Why change this logic? What are the benefits?

TimeUtils.parse(field, timeFormatter) have the best performance, but only can parse HH:mm:ss format.

I replace it with a format can parse HH:mm:ss between HH:mm:ss.SSSSSSSSS, support 0 to 9 length nano of second.

public static final DateTimeFormatter TIME_FORMAT = new DateTimeFormatterBuilder() .appendPattern("HH:mm:ss") .appendFraction(ChronoField.NANO_OF_SECOND, 0, 9, true) .toFormatter();

EricJoy2048 · 2024-04-07T09:05:26Z

Please add ut/it for DateTimeUtils and DateUtils

done.

liugddx

Please maintain the http connector document

liugddx

Do we need to add test cases for the file connector to test time formatting?

EricJoy2048 · 2024-04-08T06:30:43Z

Do we need to add test cases for the file connector to test time formatting?

I found seatunnel-e2e/seatunnel-connector-v2-e2e/connector-file-local-e2e/src/test/resources/text/local_file_text_lzo_to_assert.conf have the timestamp datatype and date datetype.

EricJoy2048 · 2024-04-08T06:44:19Z

Please maintain the http connector document

This pr has no connector parameters or usage changes and does not require updating the http documentation.

liugddx

LGTM

Carl-Zhou-CN · 2024-04-08T11:46:21Z

...main/java/org/apache/seatunnel/connectors/seatunnel/file/source/reader/TextReadStrategy.java

-                        .dateFormatter(dateFormat)
-                        .dateTimeFormatter(datetimeFormat)
-                        .timeFormatter(timeFormat);
+                TextDeserializationSchema.builder().delimiter(fieldDelimiter);


@EricJoy2048 This seems to prevent users from customizing the data format

@EricJoy2048 This seems to prevent users from customizing the data format

For reading the contents of the file, the format of the custom read is meaningless, because if the user-defined format is not consistent with the date and time format in the file, it will fail to read, so the format must be consistent with the content format in the file under the reading scenario.

For scenarios where a date or time is written to a file, a user-defined format makes sense.So, I am not change the TextWriteStrategy code.

Carl-Zhou-CN

LGTM

EricJoy2048 force-pushed the 240328_fix_http_source_bug branch from b2c39bb to 3f44f9c Compare March 29, 2024 07:09

liunaijie reviewed Apr 3, 2024

View reviewed changes

TyrantLucifer reviewed Apr 3, 2024

View reviewed changes

EricJoy2048 force-pushed the 240328_fix_http_source_bug branch from c44202c to 2a97ec0 Compare April 7, 2024 08:54

EricJoy2048 force-pushed the 240328_fix_http_source_bug branch from 336ba78 to 4046531 Compare April 7, 2024 09:12

EricJoy2048 requested review from TyrantLucifer and liunaijie April 7, 2024 09:13

liugddx reviewed Apr 8, 2024

View reviewed changes

liugddx previously approved these changes Apr 8, 2024

View reviewed changes

github-actions bot added approved reviewed labels Apr 8, 2024

Carl-Zhou-CN reviewed Apr 8, 2024

View reviewed changes

EricJoy2048 added 7 commits April 9, 2024 10:22

Fix http source bug

376076f

Improve datetime utils

5df8960

code format

bf2b2f3

Fix build error

5201250

move main method to ut

4e11b64

Add DateUtils UT

0d85176

rebase from dev

979c175

EricJoy2048 dismissed liugddx’s stale review via 979c175 April 9, 2024 02:27

EricJoy2048 force-pushed the 240328_fix_http_source_bug branch from 4046531 to 979c175 Compare April 9, 2024 02:27

github-actions bot removed approved reviewed labels Apr 9, 2024

Carl-Zhou-CN approved these changes Apr 9, 2024

View reviewed changes

github-actions bot added the approved label Apr 9, 2024

github-actions bot added the reviewed label Apr 9, 2024

liugddx approved these changes Apr 9, 2024

View reviewed changes

liugddx merged commit 19888e7 into apache:dev Apr 9, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Hotfix] fix http source can not read yyyy-MM-dd HH:mm:ss format bug & Improve DateTime Utils #6601

[Hotfix] fix http source can not read yyyy-MM-dd HH:mm:ss format bug & Improve DateTime Utils #6601

EricJoy2048 commented Mar 28, 2024 •

edited

liunaijie Apr 3, 2024

TyrantLucifer Apr 3, 2024

EricJoy2048 Apr 7, 2024

EricJoy2048 Apr 7, 2024

TyrantLucifer left a comment

TyrantLucifer Apr 3, 2024

TyrantLucifer Apr 3, 2024

EricJoy2048 Apr 7, 2024

EricJoy2048 commented Apr 7, 2024

liugddx left a comment

liugddx left a comment

EricJoy2048 commented Apr 8, 2024

EricJoy2048 commented Apr 8, 2024

liugddx left a comment

Carl-Zhou-CN Apr 8, 2024

EricJoy2048 Apr 9, 2024

Carl-Zhou-CN Apr 9, 2024

Carl-Zhou-CN left a comment

[Hotfix] fix http source can not read yyyy-MM-dd HH:mm:ss format bug & Improve DateTime Utils #6601

[Hotfix] fix http source can not read yyyy-MM-dd HH:mm:ss format bug & Improve DateTime Utils #6601

Conversation

EricJoy2048 commented Mar 28, 2024 • edited

Purpose of this pull request

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TyrantLucifer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EricJoy2048 commented Apr 7, 2024

liugddx left a comment

Choose a reason for hiding this comment

liugddx left a comment

Choose a reason for hiding this comment

EricJoy2048 commented Apr 8, 2024

EricJoy2048 commented Apr 8, 2024

liugddx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Carl-Zhou-CN left a comment

Choose a reason for hiding this comment

EricJoy2048 commented Mar 28, 2024 •

edited