[BEAM-10702] Do not implicitly decompress artifacts

[jkff]

Context: https://issues.apache.org/jira/browse/BEAM-10702

Looks like the artifact service started with --job_endpoint=embed uses FileSystems.open to read the artifacts to be staged, which by default decompresses the artifacts if they are compressed. PIP packages are compressed as .tar.gz, so when the worker container retrieves the PIP packages to install, they see .tar.gz files whose contents is actually the underlying .tar file.

I would appreciate guidance for where to add tests for this.

• artifact_service_test uses an in-memory file reader fn that doesn't actually share the code being used in production. Should I change that?
• worker_handlers_test does not exist - there's fn_runner_test.py in that directory but I'm not sure how to test the artifact staging codepath via there.
• Do we have any ValidatesRunner tests at all with job_endpoint=embed? If not, how would I go about adding some?

R: @robertwb@pabloem

Post-Commit Tests Status (on master branch)

Lang	SDK	Dataflow	Flink	Samza	Spark	Twister2
Go		---		---		---
Java
Python				---		---
XLang		---		---		---

Pre-Commit Tests Status (on master branch)

---	Java	Python	Go	Website
Non-portable
Portable	---		---	---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI.

[tvalentyn]

@robertwb@ihji@chamikaramj - do you know what is the status of Portable local runner tests? It sounds like the codepath with the bug that @jkff discovered has no test coverage. Do you have a recommendation or plan to improve testing?

[jkff]

FWIW: I can reproduce this with the local Flink runner too:
python3 main.py --runner=FlinkRunner --environment_type=DOCKER --requirements_file=requirements.txt for the main.py in the thread linked from JIRA - this gives me the same issue.

Are there any tests running with these settings?

Note that the issue reproduces only if there are any compressed artifacts to stage: either a non-empty requirements.txt (then the dependencies are staged) or a setup.py (then we stage workflow.tar.gz)

Maybe there already are some tests, and we can make them have a requirements.txt or setup.py?

[jkff]

Looks like we do have a test with a setup.py - it's the Julia Set example. I'll try to add that as a test.

[tvalentyn]

Are there any tests running with these settings?

Likely not exactly, since if we had such tests, they should have been red.

We do exercise --requirements_file=requirements.txt in Dataflow Postcommits, but those postcommits don't exercise the artifact service Python implementation.

I don't think we have a test that exercises Julia Set example as an integration test either (BEAM-10639), so that would be a great addition in test coverage.

[tvalentyn]

Current changes LGTM.

[jkff]

Run Python 3.8 PostCommit

[jkff]

Thanks @tvalentyn ! I added a Julia Set test that reproduces the issue and passes with the fix.

[jkff]

(PVR_Flink is already failing - not caused by this PR)

[jkff]

The python precommit test failure is also spurious - unrelated to the PR, and passes for me locally.

[tvalentyn]

I suggest moving this to

beam/sdks/python/test-suites/portable/common.gradle

Line 183 in 0f04de7

':runners:spark:job-server:shadowJar',

[jkff]

Thanks @tvalentyn ! Addressed your comment, will merge.