-
Notifications
You must be signed in to change notification settings - Fork 4.5k
[BEAM-6527] Use Gradle to parallel Python tox tests #7675
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
Conversation
|
Run Python PreCommit |
4 similar comments
|
Run Python PreCommit |
|
Run Python PreCommit |
|
Run Python PreCommit |
|
Run Python PreCommit |
|
One of tox test keep failing due to The build execute multiple tox commands in parallel so it's possibly a race condition. However, I set Triggered this test few tests and consistently failed regardless of beam nodes. Out of idea how to move forward. |
a2d80fd to
3c6979e
Compare
| project.exec { | ||
| executable 'sh' | ||
| args '-c', ". ${project.ext.envdir}/bin/activate && cd ${pythonRootDir} && python setup.py sdist --keep-temp --formats zip,gztar --dist-dir ${project.buildDir}" | ||
| args '-c', ". ${project.ext.envdir}/bin/activate && cd ${copiedSrcDir}/sdks/python && python setup.py sdist --formats zip,gztar --dist-dir ${project.buildDir}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we start to use copy of sdk sources to build artifact, there is no need to keep temp directory around after build. So remove --keep-temp flag here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For my education, why did we need it before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this flag to fix the parallel failure in integration tests since the temp directory is shared between different build processes and by default it's deleted after a build finish. However, it never works for tox tests.
|
+R: @tvalentyn @charlesccychen |
buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy
Outdated
Show resolved
Hide resolved
| project.exec { | ||
| executable 'sh' | ||
| args '-c', ". ${project.ext.envdir}/bin/activate && cd ${pythonRootDir} && python setup.py sdist --keep-temp --formats zip,gztar --dist-dir ${project.buildDir}" | ||
| args '-c', ". ${project.ext.envdir}/bin/activate && cd ${copiedSrcDir}/sdks/python && python setup.py sdist --formats zip,gztar --dist-dir ${project.buildDir}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For my education, why did we need it before?
|
PTAL @tvalentyn |
|
Run Python PreCommit |
|
Is this ready for review, @markflyhigh ? |
|
Yes, please take a look. @tvalentyn |
tvalentyn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! This is great!
|
R: @aaltay |
|
Thank you! |
|
BTW, the reason why I choose copy SDK sources to isolate build is explained in #7793 (comment). |
This PR use Gradle to parallel Python 2 and 3 tox as well as integration tests (on DataflowRunner) in Python PreCommit by:
setup.py(details: Set sdk_location for Python 3 integration tests #7793 (comment))Reduced ~30mins for the PreCommit build:
Follow this checklist to help us incorporate your contribution quickly and easily:
[BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replaceBEAM-XXXwith the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.It will help us expedite review of your Pull Request if you tag someone (e.g.
@username) to look at it.Post-Commit Tests Status (on master branch)