[FLINK-22519][flink-python] support tar python archives #15813

YikSanChan · 2021-04-30T02:10:05Z

What is the purpose of the change

Support tar python archives.

Brief change log

Add TarUtils.
Choose between ZipUtils vs. TarUtils based on the archivePath.
TODO: tests and docs.

Verifying this change

This change added tests and can be verified as follows: TODO

Does this pull request potentially affect one of the following parts:

Dependencies (does it add or upgrade a dependency): no
The public API, i.e., is any changed class annotated with @Public(Evolving): no
The serializers: no
The runtime per-record code paths (performance sensitive): no
Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn/Mesos, ZooKeeper: no
The S3 file system connector: no

Documentation

Does this pull request introduce a new feature? yes
If yes, how is the feature documented? docs and JavaDocs

flinkbot · 2021-04-30T02:12:21Z

Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.

Automated Checks

Last check on commit ec561cd (Fri Apr 30 02:12:21 UTC 2021)

Warnings:

No documentation files were touched! Remember to keep the Flink docs up to date!

_{Mention the bot in a comment to re-run the automated checks.}

Review Progress

❓ 1. The [description] looks good.
❓ 2. There is [consensus] that the contribution should go into to Flink.
❓ 3. Needs [attention] from.
❓ 4. The change fits into the overall [architecture].
❓ 5. Overall code [quality] is good.

Please see the Pull Request Review Guide for a full explanation of the review process.

The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot approve description to approve one or more aspects (aspects: description, consensus, architecture and quality)
@flinkbot approve all to approve all aspects
@flinkbot approve-until architecture to approve everything until architecture
@flinkbot attention @username1 [@username2 ..] to require somebody's attention
@flinkbot disapprove architecture to remove an approval you gave earlier

flinkbot · 2021-04-30T02:28:06Z

CI report:

1533b82 Azure: FAILURE

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot run travis re-run the last Travis build
@flinkbot run azure re-run the last Azure build

flink-python/src/main/java/org/apache/flink/python/util/TarGzUtils.java

...k-python/src/main/java/org/apache/flink/python/env/beam/ProcessPythonEnvironmentManager.java

YikSanChan · 2021-05-10T09:56:53Z

flink-python/src/main/java/org/apache/flink/python/util/TarUtils.java

+        String untarCommand =
+                gzipped
+                        ? String.format(
+                                "gzip -dc '%s' | (cd '%s' && tar -xf -)", inFilePath, targetDirPath)


I wonder if I can use tar -xzf directly? It is simpler, but I am not exactly sure why hadoop-common chooses to do the more verbose way, i.e., gzip ... | tar -xf ...

...k-python/src/main/java/org/apache/flink/python/env/beam/ProcessPythonEnvironmentManager.java

flink-python/src/main/java/org/apache/flink/python/util/DecompressUtils.java

flink-python/src/main/java/org/apache/flink/python/util/TarUtils.java

dianfu · 2021-05-10T11:51:50Z

flink-python/src/main/java/org/apache/flink/python/util/TarUtils.java

+                    if (!f.isDirectory() && !f.mkdirs()) {
+                        throw new IOException("failed to create directory " + f);
+                    }
+                } else {


It also considered link and hard link in https://github.com/apache/hadoop/blob/7f93349ee74da5f35276b7535781714501ab2457/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileUtil.java#L991. Should we also consider that?

dianfu · 2021-05-10T12:26:56Z

@YikSanChan Thanks a lot for the update. Have left a few comments as above. Besides, it would be great if we could add some tests.

Co-authored-by: Dian Fu <dianfu@apache.org>

zjffdu · 2021-06-25T20:15:47Z

Any update on this PR ? Supporting tar is definitely necessary.

YikSanChan · 2021-06-25T23:27:31Z

Any update on this PR ? Supporting tar is definitely necessary.

Hi Jeff, I haven't worked on this for a while. Though if you see the need being urgent, I am happy to revisit and try to finish this next week.

zjffdu · 2021-06-25T23:56:03Z

Thanks @YikSanChan I am trying to make a workaround via yarn.ship-archives, but it would still be helpful if this PR is merged.

dianfu · 2021-06-30T02:44:48Z

@YikSanChan Regarding to tests, you could take a look ZipUtilsTests which is added recently for reference.

dianfu · 2021-07-16T02:14:22Z

@YikSanChan What's the status of this PR? I'm asking because we are approaching the feature freeze of 1.14 which is planned at the end of this month. It would be great to include this feature in 1.14.

zjffdu · 2021-07-16T02:48:00Z

@YikSanChan @dianfu Here's one zeppelin notebook which create conda env tar and zip file to use customized python env in yarn cluster. But it would be great if this PR can be merged so that I can only create one tar file. http://23.254.161.240/#/notebook/2G8N1WTTS

YikSanChan · 2021-07-16T08:17:11Z

@dianfu @zjffdu my bad since I wasn't working on this for a while. Luckily I will get some free time next week, I will try and let you folks know by EOW next week

zjffdu · 2021-07-26T12:10:06Z

@YikSanChan Any update ?

YikSanChan · 2021-07-28T00:51:33Z

@YikSanChan Any update ?

Hi Jeff, unfortunately I am not able to work on this recently.

dianfu · 2021-08-16T05:47:10Z

@YikSanChan Since this feature is very important and you don't have much bandwidth on this work. If you don't mind, I'd like to take over this PR and continue the work based on this PR.

YikSanChan · 2021-08-16T05:56:32Z

@YikSanChan Since this feature is very important and you don't have much bandwidth on this work. If you don't mind, I'd like to take over this PR and continue the work based on this PR.

Feel free, no worry at all.

dianfu · 2021-08-16T07:57:35Z

@YikSanChan Thanks a lot~

This closes apache#15813.

rmetzger added the review=description? label Apr 30, 2021

rmetzger added the component=API/Python label Apr 30, 2021

dianfu reviewed Apr 30, 2021

View reviewed changes

YikSanChan force-pushed the 22519-support-targz-python-archives branch from ec561cd to 0d44263 Compare May 10, 2021 07:46

YikSanChan changed the title ~~[FLINK-22519][flink-python] support tar.gz python archives~~ [FLINK-22519][flink-python] support tar python archives May 10, 2021

YikSanChan commented May 10, 2021

View reviewed changes

dianfu reviewed May 10, 2021

View reviewed changes

YikSanChan and others added 13 commits May 11, 2021 19:58

Add TarGzUtils

70e19b9

dedup code between ZipUtils and TarGzUtils

229c1ca

Support .tar.gz suffix

ae65427

untar using tar on unix

c6edb80

touch ZipUtils less

814b881

docs

c74a750

throws Exception

9d5a766

Apply suggestions from code review

e8441db

Co-authored-by: Dian Fu <dianfu@apache.org>

Move decompress methods to DecompressUtils

c11b1d7

check suffix of lowercased file path

8a1d875

add license header

8e4e72b

Not OOM when entry is large

31a05df

format

1533b82

YikSanChan force-pushed the 22519-support-targz-python-archives branch from 180a9c6 to 1533b82 Compare May 11, 2021 11:58

dianfu closed this in ab7f541 Aug 16, 2021

hhkkxxx133 pushed a commit to hhkkxxx133/flink that referenced this pull request Aug 25, 2021

[FLINK-22519][python] Support tgz files as python archives

6a31748

This closes apache#15813.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FLINK-22519][flink-python] support tar python archives #15813

[FLINK-22519][flink-python] support tar python archives #15813

YikSanChan commented Apr 30, 2021 •

edited

flinkbot commented Apr 30, 2021

flinkbot commented Apr 30, 2021 •

edited

YikSanChan May 10, 2021

dianfu May 10, 2021

dianfu commented May 10, 2021

zjffdu commented Jun 25, 2021

YikSanChan commented Jun 25, 2021

zjffdu commented Jun 25, 2021

dianfu commented Jun 30, 2021

dianfu commented Jul 16, 2021

zjffdu commented Jul 16, 2021

YikSanChan commented Jul 16, 2021

zjffdu commented Jul 26, 2021

YikSanChan commented Jul 28, 2021 •

edited

dianfu commented Aug 16, 2021

YikSanChan commented Aug 16, 2021

dianfu commented Aug 16, 2021

[FLINK-22519][flink-python] support tar python archives #15813

[FLINK-22519][flink-python] support tar python archives #15813

Conversation

YikSanChan commented Apr 30, 2021 • edited

What is the purpose of the change

Brief change log

Verifying this change

Does this pull request potentially affect one of the following parts:

Documentation

flinkbot commented Apr 30, 2021

Automated Checks

Review Progress

flinkbot commented Apr 30, 2021 • edited

CI report:

YikSanChan May 10, 2021

Choose a reason for hiding this comment

dianfu May 10, 2021

Choose a reason for hiding this comment

dianfu commented May 10, 2021

zjffdu commented Jun 25, 2021

YikSanChan commented Jun 25, 2021

zjffdu commented Jun 25, 2021

dianfu commented Jun 30, 2021

dianfu commented Jul 16, 2021

zjffdu commented Jul 16, 2021

YikSanChan commented Jul 16, 2021

zjffdu commented Jul 26, 2021

YikSanChan commented Jul 28, 2021 • edited

dianfu commented Aug 16, 2021

YikSanChan commented Aug 16, 2021

dianfu commented Aug 16, 2021

YikSanChan commented Apr 30, 2021 •

edited

flinkbot commented Apr 30, 2021 •

edited

YikSanChan commented Jul 28, 2021 •

edited