[BEAM-13183] [BEAM-8152] Use venv instead of virtualenv to create Python environments in Gradle scripts. #15819

tvalentyn · 2021-10-27T23:17:52Z

Remove a dependency on virtualenv, instead use venv, which is a part of Python library (https://docs.python.org/3/library/venv.html).

Use venv in gradle commands and in the website documentation.

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.
Update CHANGES.md with noteworthy changes.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

`ValidatesRunner` compliance status (on master branch)

Lang	ULR	Twister2
Go	---	---
Java
Python	---	---
XLang		---

Examples testing status on various runners

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

Post-Commit SDK/Transform Integration Tests Status (on master branch)

Go	Java	Python

Pre-Commit Tests Status (on master branch)

---	Java	Python	Go	Website	Whitespace	Typescript
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.

codecov · 2021-10-27T23:32:32Z

Codecov Report

Merging #15819 (e87f846) into master (2335ec6) will decrease coverage by 0.02%.
The diff coverage is n/a.

❗ Current head e87f846 differs from pull request most recent head 0a5ec87. Consider uploading reports for the commit 0a5ec87 to get more accurate results

@@            Coverage Diff             @@
##           master   #15819      +/-   ##
==========================================
- Coverage   83.63%   83.60%   -0.03%     
==========================================
  Files         445      445              
  Lines       61371    61329      -42     
==========================================
- Hits        51326    51277      -49     
- Misses      10045    10052       +7

Impacted Files	Coverage Δ
...hon/apache_beam/runners/direct/test_stream_impl.py	`94.02% <0.00%> (-2.24%)`	⬇️
...pache_beam/runners/interactive/interactive_beam.py	`75.60% <0.00%> (-0.98%)`	⬇️
sdks/python/apache_beam/runners/direct/executor.py	`96.46% <0.00%> (-0.55%)`	⬇️
...runners/interactive/display/pcoll_visualization.py	`85.85% <0.00%> (-0.51%)`	⬇️
sdks/python/apache_beam/transforms/combiners.py	`93.15% <0.00%> (-0.40%)`	⬇️
...eam/runners/interactive/interactive_environment.py	`91.37% <0.00%> (-0.35%)`	⬇️
sdks/python/apache_beam/typehints/typehints.py	`93.34% <0.00%> (-0.10%)`	⬇️
sdks/python/apache_beam/coders/coders.py	`88.61% <0.00%> (-0.03%)`	⬇️
sdks/python/apache_beam/pipeline.py	`91.73% <0.00%> (-0.02%)`	⬇️
sdks/python/apache_beam/transforms/core.py	`92.63% <0.00%> (-0.01%)`	⬇️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2335ec6...0a5ec87. Read the comment docs.

tvalentyn · 2021-10-29T18:41:03Z

Run Python_PVR_Flink PreCommit

tvalentyn · 2021-10-29T18:43:05Z

Run Python_PVR_Flink PreCommit

tvalentyn · 2021-10-29T18:48:07Z

Run PythonDocs PreCommit

tvalentyn · 2021-10-29T19:24:33Z

Run Python_PVR_Flink PreCommit

tvalentyn · 2021-10-29T19:27:59Z

Run Python_PVR_Flink PreCommit

tvalentyn · 2021-10-29T19:28:09Z

Run Python PreCommit

tvalentyn · 2021-10-29T23:46:23Z

Run Python_PVR_Flink PreCommit

tvalentyn · 2021-10-29T23:46:33Z

Run Python PreCommit

tvalentyn · 2021-10-29T23:46:44Z

Run Portable_Python PreCommit

tvalentyn · 2021-10-29T23:48:33Z

Run GoPortable PreCommit

tvalentyn · 2021-10-30T01:11:11Z

Run Python PreCommit

tvalentyn · 2021-10-30T01:11:32Z

Run Portable_Python PreCommit

tvalentyn · 2021-11-03T17:37:08Z

Run Python PreCommit

tvalentyn · 2021-11-03T17:37:19Z

Run Portable_Python PreCommit

tvalentyn · 2021-11-03T17:37:36Z

Run Website PreCommit

tvalentyn · 2021-11-03T17:37:51Z

Run GoPortable PreCommit

tvalentyn · 2021-11-03T17:37:58Z

Run Java PreCommit

tvalentyn · 2021-11-03T17:38:51Z

Run Python_PVR_Flink PreCommit

tvalentyn · 2021-11-03T17:38:59Z

Run SQL PreCommit

tvalentyn · 2021-11-12T16:37:16Z

r: @ryanthompson591 @TheNeuralBit

TheNeuralBit

This LGTM, I have a few questions though

TheNeuralBit · 2021-11-17T00:00:44Z

website/www/site/content/en/contribute/_index.md

-      - pip, setuptools, virtualenv, and tox installed for Python development
+      - Python 3.x interpreters. You will need Python interpreters for all Python versions supported by Beam.
+        Interpreters should be installed and available in shell via `python3.x` commands. For more information, see:
+        Python installation tips in [Developer Wiki](https://cwiki.apache.org/confluence/display/BEAM/Python+Tips#PythonTips-InstallingPythoninterpreters).


Consider noting the issue with venv in deadsnakes pythons, and the need to install python3.x-dev

I added a note to the Wiki about it.

TheNeuralBit · 2021-11-17T00:01:30Z

dev-support/docker/pkglist

 tox
 docker.io
+
+
+python3.6 python3.6-dev python3.6-venv python3.7 python3.7-dev python3.7-distutils python3.7-venv python3.8 python3.8-dev  python3.8-distutils python3.8-venv


what is this line?

I think there is some 'dev' environment, and this file lists all apt packages to install in that environment.

ah looks like the line is a leftover, thanks for catching.

re: dev envirornment - this file is referenced from ./local-env-setup.sh (configures current system), and ./start-build-env.sh (builds and start a development container)

TheNeuralBit · 2021-11-17T00:04:34Z

dev-support/docker/pkglist

@@ -33,7 +33,22 @@ openjdk-8-jdk
 python3-setuptools
 python3-pip
 python3.6
+python3.6-dev
+python3.6-venv


Should we drop 3.6? (also note python is aliased to python3.6 in dev-support/docker/Dockerfile)

it's to be dropped in Beam 2.36.0 (after 2.35.0 release cut)

I see, that makes sense. I thought maybe we were already dropping it because of the for ver in 3.7 3.8 3.9; do in local-env-setup.sh

TheNeuralBit · 2021-11-17T00:05:51Z

dev-support/docker/Dockerfile

@@ -66,7 +66,7 @@ RUN alias python=python3.6
 ###
 # Install grpcio-tools mypy-protobuf for `python3 sdks/python/setup.py sdist` to work
 ###
-RUN pip3 install grpcio-tools mypy-protobuf virtualenv
+RUN pip3 install grpcio-tools mypy-protobuf

 ###
 # Install useful tools


Maybe we can drop distilib below? It says it's installed to workaround a virtualenv issue

I believe I tried to remove it and still ran into some issues. We can try again later.

TheNeuralBit · 2021-11-17T00:40:45Z

local-env-setup.sh

+      fi
+      if [ ! $(type -P python$ver) > /dev/null 2>&1 ]; then
+        # For some python packages, brew does not add symlinks...
+        # TODO: use pyenv.


What exactly is the TODO here? Will pyenv make symlinks for us?

Reworded to: # TODO: Consider using pyenv to manage multiple installations of Python.

TheNeuralBit · 2021-11-17T00:47:04Z

sdks/python/build-requirements.txt

@@ -14,6 +14,12 @@
 #    See the License for the specific language governing permissions and
 #    limitations under the License.
 #
+
+# TODO: Consider PEP-517/PEP-518 instead of this file (BEAM-8954).


nit:

Suggested change

# TODO: Consider PEP-517/PEP-518 instead of this file (BEAM-8954).

# TODO(BEAM-8954): Consider PEP-517/PEP-518 instead of this file.

TheNeuralBit · 2021-11-17T00:47:37Z

sdks/python/build-requirements.txt

+# TODO: Consider PEP-517/PEP-518 instead of this file (BEAM-8954).
+
+setuptools
+wheel>=0.36.0


why is this change necessary?

these are common build dependencies, however wheel is not installed by default into venv, but is installed in virtualenv-created env. without this change, installing from sdist was failing on some python versions.

setuptools is still implicitly installed, keeping it explicitly so that we add them when we adopt PEP-517/518 which might require them mentioned explicitly.

TheNeuralBit · 2021-11-17T00:49:38Z

.test-infra/jenkins/dependency_check/generate_report.sh


-python -m dependency_check.dependency_check_report_generator Java
+$PYTHON -m dependency_check.dependency_check_report_generator Java


Were you able to verify this (and other .test-infra changes)?

seems to work, thanks.

TheNeuralBit · 2021-11-17T00:54:58Z

release/src/main/scripts/run_rc_validation.sh

@@ -648,7 +646,7 @@ if [[ ("$python_xlang_kafka_taxi_dataflow" = true
  do
    rm -rf ./beam_env_${py_version}
    echo "--------------Setting up virtualenv with $py_version interpreter----------------"
-    virtualenv beam_env_${py_version} -p $py_version
+    $py_version -m venv beam_env_${py_version}


Same question about the release script changes, but fortunately you are the next release manager, so I trust you'll be able to detect and fix any issues here.

TheNeuralBit · 2021-11-17T00:56:23Z

sdks/java/container/build.gradle

@@ -22,6 +22,7 @@ plugins {
 }

 applyGoNature()
+applyPythonNature()


Maybe add a comment that this is needed for ./license_scripts/license_script.sh. Also, why is it that we need this now, but it seemed to work ok before?

lukecwik · 2021-11-17T16:21:03Z

.test-infra/jenkins/dependency_check/generate_report.sh

@@ -20,6 +20,9 @@
 set -e
 set -v

+# Get currently used Python version from args or assume a default.
+PYTHON=${1:-python3}


Does this work if users are using an alias/symlink?

yes, it should work. python3 is typically a symlink.

alias probably wouldn't work

verified that symlinks work and aliases don't

tvalentyn · 2021-11-18T04:48:13Z

Run Seed Job

tvalentyn · 2021-11-18T04:54:24Z

Run Inventory apache-beam-jenkins-1

tvalentyn · 2021-11-18T04:54:51Z

Run Dependency Check

…can be avoided.

tvalentyn changed the title ~~[BEAM-12000] Use venv instead of virtualenv to avoid a virtualenv dependency that can be avoided.~~ [BEAM-8152] Use venv instead of virtualenv to avoid a virtualenv dependency that can be avoided. Oct 27, 2021

tvalentyn force-pushed the venv branch from 30e6fd1 to 177e50e Compare October 27, 2021 23:46

tvalentyn force-pushed the venv branch 2 times, most recently from be27073 to 0b9e8e2 Compare November 12, 2021 06:51

tvalentyn changed the title ~~[BEAM-8152] Use venv instead of virtualenv to avoid a virtualenv dependency that can be avoided.~~ [BEAM-8152] Use venv instead of virtualenv to create Python environments on Gradle. Nov 12, 2021

tvalentyn changed the title ~~[BEAM-8152] Use venv instead of virtualenv to create Python environments on Gradle.~~ [BEAM-8152] Use venv instead of virtualenv to create Python environments in Gradle scripts. Nov 12, 2021

tvalentyn mentioned this pull request Nov 16, 2021

[BEAM-13264] Allow pyarrow up to 6.x #15995

Merged

TheNeuralBit self-requested a review November 16, 2021 21:46

TheNeuralBit mentioned this pull request Nov 17, 2021

[BEAM-13624] Run generatePythonRequirementsAll for pyarrow 6 #16000

Merged

TheNeuralBit approved these changes Nov 17, 2021

View reviewed changes

lukecwik reviewed Nov 17, 2021

View reviewed changes

tvalentyn changed the title ~~[BEAM-8152] Use venv instead of virtualenv to create Python environments in Gradle scripts.~~ [BEAM-13183] [BEAM-8152] Use venv instead of virtualenv to create Python environments in Gradle scripts. Nov 18, 2021

tvalentyn added 6 commits November 17, 2021 22:04

Use venv instead of virtualenv to avoid a virtualenv dependency that …

c2e979e

…can be avoided.

Add wheel as build dependency, use a version that supports Py3.9.

b36c9ee

Parameterize shell scripts triggered by jenkins with Python verson.

108292d

Parameterize website build config with Python version.

2ed45a2

Update GH actions and local Docker dev environments.

84a9012

Address review feedback.

0a5ec87

tvalentyn force-pushed the venv branch from 0b9e8e2 to 0a5ec87 Compare November 18, 2021 06:04

tvalentyn merged commit 3f504c7 into apache:master Nov 18, 2021

tvalentyn deleted the venv branch November 18, 2021 06:07

tvalentyn mentioned this pull request Dec 29, 2021

Update venv creation commands in release scripts. #16381

Merged

	# TODO: Consider PEP-517/PEP-518 instead of this file (BEAM-8954).
	# TODO(BEAM-8954): Consider PEP-517/PEP-518 instead of this file.


		python -m dependency_check.dependency_check_report_generator Java
		$PYTHON -m dependency_check.dependency_check_report_generator Java

[BEAM-13183] [BEAM-8152] Use venv instead of virtualenv to create Python environments in Gradle scripts. #15819

[BEAM-13183] [BEAM-8152] Use venv instead of virtualenv to create Python environments in Gradle scripts. #15819

Conversation

tvalentyn commented Oct 27, 2021 • edited Loading

ValidatesRunner compliance status (on master branch)

Examples testing status on various runners

Post-Commit SDK/Transform Integration Tests Status (on master branch)

Pre-Commit Tests Status (on master branch)

GitHub Actions Tests Status (on master branch)

codecov bot commented Oct 27, 2021 • edited Loading

Codecov Report

tvalentyn commented Oct 29, 2021

tvalentyn commented Oct 29, 2021

tvalentyn commented Oct 29, 2021

tvalentyn commented Oct 29, 2021

tvalentyn commented Oct 29, 2021

tvalentyn commented Oct 29, 2021

tvalentyn commented Oct 29, 2021

tvalentyn commented Oct 29, 2021

tvalentyn commented Oct 29, 2021

tvalentyn commented Oct 29, 2021

tvalentyn commented Oct 30, 2021

tvalentyn commented Oct 30, 2021

tvalentyn commented Nov 3, 2021

tvalentyn commented Nov 3, 2021

tvalentyn commented Nov 3, 2021

tvalentyn commented Nov 3, 2021

tvalentyn commented Nov 3, 2021

tvalentyn commented Nov 3, 2021

tvalentyn commented Nov 3, 2021

tvalentyn commented Nov 12, 2021

TheNeuralBit left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheNeuralBit Nov 17, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tvalentyn Nov 18, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tvalentyn commented Nov 18, 2021

tvalentyn commented Nov 18, 2021

tvalentyn commented Nov 18, 2021

tvalentyn commented Oct 27, 2021 •

edited

Loading

`ValidatesRunner` compliance status (on master branch)

codecov bot commented Oct 27, 2021 •

edited

Loading

TheNeuralBit Nov 17, 2021 •

edited

Loading

tvalentyn Nov 18, 2021 •

edited

Loading