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.

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

[Feature] support LZO compress on File Read #5083

Merged
merged 21 commits into from
Oct 16, 2023

Conversation

liunaijie
Copy link
Member

@liunaijie liunaijie commented Jul 14, 2023

Purpose of this pull request

support compress on local text file read.

Check list

@TyrantLucifer
Copy link
Member

Good pr, let's waiting CI/CD

Copy link
Contributor

@TaoZex TaoZex left a comment

Choose a reason for hiding this comment

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

LGTM
But the doc needs to be modified.

@liunaijie
Copy link
Member Author

LGTM But the doc needs to be modified.

doc updated.

@liunaijie liunaijie changed the title [Feature] support compress on Text File Read [Feature] support compress on File Read Jul 20, 2023
@liunaijie
Copy link
Member Author

liunaijie commented Jul 20, 2023

updated json reader.
updated document

@liunaijie
Copy link
Member Author

@TaoZex @TyrantLucifer PTAL

@liunaijie
Copy link
Member Author

pls re-trigger the CI as i update more formart reader

@TyrantLucifer
Copy link
Member

pls re-trigger the CI as i update more formart reader

Done, let's waiting CI/CD.

@liunaijie
Copy link
Member Author

pls trigger the CI

# Conflicts:
#	docs/en/connector-v2/source/LocalFile.md
#	seatunnel-connectors-v2/connector-file/connector-file-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/file/config/BaseSourceConfig.java
#	seatunnel-e2e/seatunnel-connector-v2-e2e/connector-file-local-e2e/src/test/java/org/apache/seatunnel/e2e/connector/file/local/LocalFileIT.java
@liunaijie
Copy link
Member Author

@TyrantLucifer @TaoZex PTAL

@ic4y
Copy link
Contributor

ic4y commented Aug 5, 2023

The documentation for connectors depending on the file-base package, such as hdfsFile, oss, and s3, likely needs updating.

@liunaijie
Copy link
Member Author

The documentation for connectors depending on the file-base package, such as hdfsFile, oss, and s3, likely needs updating.

@ic4y i will create another pr (add test case and update doc) for connectors depending on the file-base

@ic4y
Copy link
Contributor

ic4y commented Aug 8, 2023

The documentation for connectors depending on the file-base package, such as hdfsFile, oss, and s3, likely needs updating.

@ic4y i will create another pr (add test case and update doc) for connectors depending on the file-base

It's best to complete this within this PR, as it's part of the whole.

@liunaijie
Copy link
Member Author

@ic4y updated related connector and document, PTAL, thanks

@ic4y
Copy link
Contributor

ic4y commented Aug 9, 2023

Waiting for CI/CD.

@liunaijie
Copy link
Member Author

failed on ClusterFaultToleranceTwoPipelineIT.testTwoPipelineStreamJobRestoreIn2NodeMasterDown:732 » ConditionTimeout

can you help re-trigger the CI, thanks

# Conflicts:
#	docs/en/connector-v2/source/S3File.md
@liunaijie
Copy link
Member Author

i can see the CI error has been fixed in another pr, can you help trigger this CI again, thanks.

@liunaijie
Copy link
Member Author

wait for this issue fix
#5342

# Conflicts:
#	docs/en/connector-v2/source/HdfsFile.md
#	docs/en/connector-v2/source/Hive.md
Copy link
Member

@EricJoy2048 EricJoy2048 left a comment

Choose a reason for hiding this comment

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

I am sorry, In the Apache project, it is not allowed to publish binary files in the source code package (because the content of the binary file is not visible and cannot be modified. If there is a problem in the file, no one can fix it), so an alternative solution is to generate the binary file through the program (the generated code is visible, and if there is a problem, the code can also be modified to fix the problem).

@Hisoka-X Hisoka-X added this to the 2.3.4 milestone Oct 13, 2023
@Hisoka-X
Copy link
Member

Hisoka-X commented Oct 13, 2023

@liunaijie I updated test case for this. Please take a look when you have free time.

@Hisoka-X
Copy link
Member

The test case have some bug will be fixed in #5622

@liunaijie
Copy link
Member Author

@liunaijie I updated test case for this. Please take a look when you have free time.

@Hisoka-X cool, thanks. the test case change looks good to me, but i find you change the discoverTestContainers method, look this change is only for testing, should't push.

@Hisoka-X
Copy link
Member

@liunaijie I updated test case for this. Please take a look when you have free time.

@Hisoka-X cool, thanks. the test case change looks good to me, but i find you change the discoverTestContainers method, look this change is only for testing, should't push.

oh, good catch. Reverted.

@EricJoy2048 EricJoy2048 dismissed TyrantLucifer’s stale review October 16, 2023 12:14

Add data check complete.

@Hisoka-X Hisoka-X changed the title [Feature] support compress on File Read [Feature] support LZO compress on File Read Oct 16, 2023
Copy link
Member

@Hisoka-X Hisoka-X left a comment

Choose a reason for hiding this comment

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

Thanks @liunaijie . LGTM

@Hisoka-X Hisoka-X merged commit a4a1901 into apache:dev Oct 16, 2023
7 checks passed
@liunaijie liunaijie deleted the text_file_read_compress branch October 31, 2023 01:39
ruanwenjun pushed a commit to ruanwenjun/incubator-seatunnel that referenced this pull request Nov 14, 2023

---------

Co-authored-by: Jia Fan <fanjiaeminem@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants