[BEAM-6907] Simply test setup by reusing Python tarball in tox & dataflow integration tests #9277

markflyhigh · 2019-08-06T18:23:21Z

Building Python tarball concurrently in each test suite caused race condition and make postcommit flaky (see BEAM-7527).

This change let Python tarball built once by sdks:python:sdist and then used in everywhere. So in order to use this tarball in other projects, tests/other tasks needs to depends on sdks:python:sdist and projects can use dependencies to reference distTarBall from sdks:python configurations.

Tests are needed especially Dataflow VR tests which is affected by the build race condition.

+R: @tvalentyn

Update:
beam_PostCommit_Py_VR_Dataflow_PR #119 is done.

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

Choose reviewer(s) and mention them in a comment (R: @username).
Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

Post-Commit Tests Status (on master branch)

Lang	Apex	Dataflow	Gearpump	Samza
Go	---	---	---	---
Java
Python	---		---	---

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.

markflyhigh · 2019-08-06T18:40:10Z

sdks/python/test-suites/dataflow/py2/build.gradle

    def cmdArgs = project.mapToArgString([
        "test_opts": testOpts,
-        "sdk_location": "${project.buildDir}/apache-beam.tar.gz",
+        "sdk_location": files(configurations.distTarBall.files).singleFile,


This line actually can be removed for all similar integration tests sine the default sdk_location (defined in run_integration_test.sh) is pointing to the correct location. However, I left it here since people who read this configuration will know how it's set and related to :sdks:python:sdist. I'm open to discussion if you think it can be removed.

I am ok to keep this. It feels like there may be a shorter way to reference the file defined by this configuration. Did you try configurations.distTarBall? Somehow it seems to work in

beam/sdks/python/container/build.gradle

Line 46 in 28a4057

from configurations.sdkSourceTarball

I'm afraid not. We need full path of this tarball, but configurations.distTarBall returns a string configuration ':sdks:python:test-suites:dataflow:py2:distTarBall'

markflyhigh · 2019-08-06T20:48:48Z

Run Python Dataflow ValidatesRunner

markflyhigh · 2019-08-06T21:50:43Z

Glad to see ModuleNotFoundError disappeared in beam_PostCommit_Py_VR_Dataflow_PR#119 and only one test failed (among 8 suites) which seems caused by pip service flaky.

I'll go ahead to fix tox broken and also run postcommit.

markflyhigh · 2019-08-06T23:31:45Z

beam_PreCommit_Python_Commit #7937 passed. Tox tests are fixed.

markflyhigh · 2019-08-06T23:32:57Z

Run Python 2 PostCommit

markflyhigh · 2019-08-06T23:37:53Z

Run Python 3.7 PostCommit

tvalentyn

Thanks, @markflyhigh !

tvalentyn · 2019-08-07T20:14:39Z

buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy

            project.exec {
              executable 'sh'
-              args '-c', ". ${project.ext.envdir}/bin/activate && cd ${copiedPyRoot} && scripts/run_tox.sh $tox_env ${project.buildDir}/apache-beam.tar.gz"
+              args '-c', ". ${project.ext.envdir}/bin/activate && cd ${copiedPyRoot} && scripts/run_tox.sh $tox_env $distTarBall"


Unrelated to this PR - do you remember why do we pass a tarball to tox suite? It looks like we started doing that with #8067.

tox will build tarball for venv install automatically if not provided. This build depends on shared file when running in parallel and cause our test flaky. So we prebuild the tarball and pass it from --installpkg flag to avoid that issue.

tvalentyn · 2019-08-07T20:38:59Z

sdks/python/test-suites/dataflow/py2/build.gradle

    def cmdArgs = project.mapToArgString([
        "test_opts": testOpts,
-        "sdk_location": "${project.buildDir}/apache-beam.tar.gz",
+        "sdk_location": files(configurations.distTarBall.files).singleFile,


I am ok to keep this. It feels like there may be a shorter way to reference the file defined by this configuration. Did you try configurations.distTarBall? Somehow it seems to work in

beam/sdks/python/container/build.gradle

Line 46 in 28a4057

from configurations.sdkSourceTarball

tvalentyn · 2019-08-07T21:03:09Z

buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy

+      // Set run order for basic tasks.
+      // This should be called after applyPythonNature() since TaskContainer
+      // requires task instances created first before setting the order.
+      project.ext.setTaskOrder = {


Hm... this feels a bit hacky. setupVirtualEnv is already in dependsOn of installGcpTest.

As for sdks:python:sdist, is it possible to add a dependency on distTarBall configuration instead?

You are right, set setupVirtualenvrun after installGcpTest is not required in here. The main purpose is to set installGcpTest run after sdks:python:sdist in each project.

I don't know if depend on distTarBall will work or not but worth to try, so that we may be able to get rid of setTaskOrder

Seems we can do installGcpTest.mustRunAfter configurations.distTarBall. This build shows correct run order. I'll go ahead and change the code.

markflyhigh · 2019-08-07T23:18:56Z

Run Python 2 PostCommit

markflyhigh · 2019-08-08T15:34:24Z

Java PreCommit failure is not related to this change.
An integration tests failed in verification step in Python PreCommit, but passed in Python2 PostCommit #60. It would be a test flaky and this change should not affect it.
Python2 PostCommit has been failed for a while due to https://issues.apache.org/jira/browse/BEAM-7924, which is also not related to this change.

Tests are done. PTAL @tvalentyn

tvalentyn · 2019-08-08T17:15:19Z

LGTM, thanks a lot, @markflyhigh. Please run all test suites that were affected by this change before merging, I would suggest to merge this change on green and give @lukecwik a heads-up since Luke is looking into test heath ATM.

markflyhigh · 2019-08-08T17:24:21Z

Run Python PreCommit

markflyhigh · 2019-08-08T17:24:32Z

Run Python 3.7 PostCommit

markflyhigh · 2019-08-08T17:27:42Z

+cc: @lukecwik

markflyhigh · 2019-08-08T18:25:55Z

Rerun Python37_PostCommit #17 and Python_PreCommit #738, and they all passed.

lukecwik · 2019-08-08T18:36:30Z

Run Java PreCommit

lukecwik · 2019-08-08T18:36:37Z

Run Python 2 PostCommit

markflyhigh commented Aug 6, 2019

View reviewed changes

markflyhigh added 2 commits August 7, 2019 13:51

[BEAM-6907] Reuse Python tarball in integration tests

13450c4

Add missing dependency and source copy in tox test

1545120

markflyhigh force-pushed the reuse-tar-ball-2 branch from fd2892d to 89ff650 Compare August 7, 2019 20:52

tvalentyn reviewed Aug 7, 2019

View reviewed changes

Build sdk tarball before running installGcpTest task

b3c2915

markflyhigh force-pushed the reuse-tar-ball-2 branch from 89ff650 to b3c2915 Compare August 7, 2019 22:48

lukecwik approved these changes Aug 8, 2019

View reviewed changes

lukecwik merged commit b69c81a into apache:master Aug 8, 2019

markflyhigh mentioned this pull request Aug 9, 2019

[BEAM-7940] Fix beam_Release_Python_NightlySnapshot #9307

Merged

3 tasks

markflyhigh changed the title ~~[BEAM-6907] Reuse Python tarball in tox & dataflow integration tests~~ [BEAM-6907] Simply test setup by reusing Python tarball in tox & dataflow integration tests Sep 13, 2019

[BEAM-6907] Simply test setup by reusing Python tarball in tox & dataflow integration tests #9277

[BEAM-6907] Simply test setup by reusing Python tarball in tox & dataflow integration tests #9277

Uh oh!

Conversation

markflyhigh commented Aug 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Post-Commit Tests Status (on master branch)

Pre-Commit Tests Status (on master branch)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markflyhigh commented Aug 6, 2019

Uh oh!

markflyhigh commented Aug 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

markflyhigh commented Aug 6, 2019

Uh oh!

markflyhigh commented Aug 6, 2019

Uh oh!

markflyhigh commented Aug 6, 2019

Uh oh!

tvalentyn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markflyhigh Aug 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markflyhigh commented Aug 7, 2019

Uh oh!

markflyhigh commented Aug 8, 2019

Uh oh!

tvalentyn commented Aug 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

markflyhigh commented Aug 8, 2019

Uh oh!

markflyhigh commented Aug 8, 2019

Uh oh!

markflyhigh commented Aug 8, 2019

Uh oh!

markflyhigh commented Aug 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lukecwik commented Aug 8, 2019

Uh oh!

lukecwik commented Aug 8, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

markflyhigh commented Aug 6, 2019 •

edited

Loading

markflyhigh commented Aug 6, 2019 •

edited

Loading

markflyhigh Aug 7, 2019 •

edited

Loading

tvalentyn commented Aug 8, 2019 •

edited

Loading

markflyhigh commented Aug 8, 2019 •

edited

Loading