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 2021/11/17 01:00:14 UTC
[GitHub] [beam] TheNeuralBit commented on a change in pull request #15819: [BEAM-8152] Use venv instead of virtualenv to create Python environments in Gradle scripts.
TheNeuralBit commented on a change in pull request #15819:
URL: https://github.com/apache/beam/pull/15819#discussion_r750780200
##########
File path: website/www/site/content/en/contribute/_index.md
##########
@@ -80,8 +80,9 @@ detail.
changes locally.
- For SDK Development:
- [Go](https://golang.org) 1.12 or later installed for Go SDK development
- - Python 3.6, 3.7, and 3.8. Yes, you need all three versions installed.
- - 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).
Review comment:
Consider noting the issue with venv in deadsnakes pythons, and the need to install python3.x-dev
##########
File path: 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).
+
+setuptools
+wheel>=0.36.0
Review comment:
why is this change necessary?
##########
File path: dev-support/docker/pkglist
##########
@@ -33,7 +33,22 @@ openjdk-8-jdk
python3-setuptools
python3-pip
python3.6
+python3.6-dev
+python3.6-venv
Review comment:
Should we drop 3.6? (also note python is aliased to python3.6 in dev-support/docker/Dockerfile)
##########
File path: dev-support/docker/pkglist
##########
@@ -33,7 +33,22 @@ openjdk-8-jdk
python3-setuptools
python3-pip
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
+python3.9
+python3.9-dev
+python3.9-distutils
+python3.9-venv
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
Review comment:
what is this line?
##########
File path: .test-infra/jenkins/dependency_check/generate_report.sh
##########
@@ -49,14 +53,14 @@ rm -f build/dependencyUpdates/beam-dependency-check-report.txt
# Insall packages and run the unit tests of the report generator and the jira manager
pip install mock jira pyyaml
cd $WORKSPACE/src/.test-infra/jenkins
-python -m dependency_check.dependency_check_report_generator_test
-python -m jira_utils.jira_manager_test
-python -m dependency_check.version_comparer_test
+$PYTHON -m dependency_check.dependency_check_report_generator_test
+$PYTHON -m jira_utils.jira_manager_test
+$PYTHON -m dependency_check.version_comparer_test
echo "<html><body>" > $WORKSPACE/src/build/dependencyUpdates/beam-dependency-check-report.html
-python -m dependency_check.dependency_check_report_generator Python
+$PYTHON -m dependency_check.dependency_check_report_generator Python
-python -m dependency_check.dependency_check_report_generator Java
+$PYTHON -m dependency_check.dependency_check_report_generator Java
Review comment:
Were you able to verify this (and other .test-infra changes)?
##########
File path: 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
Review comment:
Maybe we can drop distilib below? It says it's installed to workaround a virtualenv issue
##########
File path: 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).
Review comment:
nit:
```suggestion
# TODO(BEAM-8954): Consider PEP-517/PEP-518 instead of this file.
```
##########
File path: 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}
Review comment:
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.
##########
File path: local-env-setup.sh
##########
@@ -85,6 +85,22 @@ elif [ "$kernelname" = "Darwin" ]; then
echo "Installing openjdk@8"
brew install openjdk@8
fi
+ for ver in 3.7 3.8 3.9; do
+ if brew ls --versions python@$ver > /dev/null; then
+ echo "python@$ver already installed. Skipping"
+ brew info python@$ver
+ else
+ echo "Installing python@$ver"
+ brew install python@$ver
+ fi
+ if [ ! $(type -P python$ver) > /dev/null 2>&1 ]; then
+ # For some python packages, brew does not add symlinks...
+ # TODO: use pyenv.
Review comment:
What exactly is the TODO here? Will pyenv make symlinks for us?
##########
File path: sdks/java/container/build.gradle
##########
@@ -22,6 +22,7 @@ plugins {
}
applyGoNature()
+applyPythonNature()
Review comment:
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?
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org