You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/08/13 19:15:35 UTC
[GitHub] [beam] jkff opened a new pull request #12571: [BEAM-10702] Do not implicitly decompress artifacts
jkff opened a new pull request #12571:
URL: https://github.com/apache/beam/pull/12571
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 | [](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/) | ---
Java | [](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
Python | [](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/) | ---
XLang | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/) | ---
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website
--- | --- | --- | --- | ---
Non-portable | [](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/)
Portable | --- | [](https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | ---
See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jkff commented on pull request #12571: [BEAM-10702] Do not implicitly decompress artifacts
Posted by GitBox <gi...@apache.org>.
jkff commented on pull request #12571:
URL: https://github.com/apache/beam/pull/12571#issuecomment-673689685
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: e.g. for that main.py, it does not reproduce without a non-empty requirements.txt, because then the pipeline has zero artifacts. However, for my production pipeline, I've also seen it fail due to decompressing `workflow.tar.gz` but I'm not sure what controls whether or not that file is created.
Maybe there already are some tests, which we can simply somehow force to have a `workflow.tar.gz`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] tvalentyn commented on a change in pull request #12571: [BEAM-10702] Do not implicitly decompress artifacts
Posted by GitBox <gi...@apache.org>.
tvalentyn commented on a change in pull request #12571:
URL: https://github.com/apache/beam/pull/12571#discussion_r470356691
##########
File path: sdks/python/test-suites/portable/common.gradle
##########
@@ -188,6 +212,7 @@ project.task("postCommitPy${pythonVersionSuffix}IT") {
dependsOn = [
'setupVirtualenv',
'installGcpTest',
+ 'portableLocalRunnerJuliaSetWithSetupPy',
Review comment:
I suggest moving this to https://github.com/apache/beam/blob/0f04de72d759a5d0d2f161fa27c3028d37c35c34/sdks/python/test-suites/portable/common.gradle#L183
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jkff commented on pull request #12571: [BEAM-10702] Do not implicitly decompress artifacts
Posted by GitBox <gi...@apache.org>.
jkff commented on pull request #12571:
URL: https://github.com/apache/beam/pull/12571#issuecomment-673749080
Thanks @tvalentyn ! I added a Julia Set test that reproduces the issue and passes with the fix.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] tvalentyn commented on pull request #12571: [BEAM-10702] Do not implicitly decompress artifacts
Posted by GitBox <gi...@apache.org>.
tvalentyn commented on pull request #12571:
URL: https://github.com/apache/beam/pull/12571#issuecomment-673715013
Current changes LGTM.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jkff commented on pull request #12571: [BEAM-10702] Do not implicitly decompress artifacts
Posted by GitBox <gi...@apache.org>.
jkff commented on pull request #12571:
URL: https://github.com/apache/beam/pull/12571#issuecomment-673748839
Run Python 3.8 PostCommit
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jkff commented on pull request #12571: [BEAM-10702] Do not implicitly decompress artifacts
Posted by GitBox <gi...@apache.org>.
jkff commented on pull request #12571:
URL: https://github.com/apache/beam/pull/12571#issuecomment-673702034
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.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] tvalentyn commented on pull request #12571: [BEAM-10702] Do not implicitly decompress artifacts
Posted by GitBox <gi...@apache.org>.
tvalentyn commented on pull request #12571:
URL: https://github.com/apache/beam/pull/12571#issuecomment-673710891
> 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](https://github.com/apache/beam/blob/70fcd57942b90345c1614839a64b506b392c5a67/sdks/python/scripts/run_integration_test.sh#L225) --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.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jkff merged pull request #12571: [BEAM-10702] Do not implicitly decompress artifacts
Posted by GitBox <gi...@apache.org>.
jkff merged pull request #12571:
URL: https://github.com/apache/beam/pull/12571
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] tvalentyn commented on pull request #12571: [BEAM-10702] Do not implicitly decompress artifacts
Posted by GitBox <gi...@apache.org>.
tvalentyn commented on pull request #12571:
URL: https://github.com/apache/beam/pull/12571#issuecomment-673681915
@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?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jkff commented on pull request #12571: [BEAM-10702] Do not implicitly decompress artifacts
Posted by GitBox <gi...@apache.org>.
jkff commented on pull request #12571:
URL: https://github.com/apache/beam/pull/12571#issuecomment-673796717
Thanks @tvalentyn ! Addressed your comment, will merge.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jkff edited a comment on pull request #12571: [BEAM-10702] Do not implicitly decompress artifacts
Posted by GitBox <gi...@apache.org>.
jkff edited a comment on pull request #12571:
URL: https://github.com/apache/beam/pull/12571#issuecomment-673689685
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?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jkff commented on pull request #12571: [BEAM-10702] Do not implicitly decompress artifacts
Posted by GitBox <gi...@apache.org>.
jkff commented on pull request #12571:
URL: https://github.com/apache/beam/pull/12571#issuecomment-673767803
The python precommit test failure is also spurious - unrelated to the PR, and passes for me 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jkff commented on pull request #12571: [BEAM-10702] Do not implicitly decompress artifacts
Posted by GitBox <gi...@apache.org>.
jkff commented on pull request #12571:
URL: https://github.com/apache/beam/pull/12571#issuecomment-673762881
(PVR_Flink is already failing - not caused by 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org