tvalentyn commented on code in PR #26156:
URL: https://github.com/apache/beam/pull/26156#discussion_r1166008342


##########
buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy:
##########
@@ -2739,7 +2739,7 @@ class BeamModulePlugin implements Plugin<Project> {
       // If none of them applied, version set here will be used as default 
value.
       // TODO(BEAM-12000): Move default value to Py3.9.
       project.ext.pythonVersion = project.hasProperty('pythonVersion') ?
-          project.pythonVersion : '3.8'
+          project.pythonVersion : '3.10'

Review Comment:
   for my education - is this a driveby change or this was necessary?



##########
buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy:
##########
@@ -2739,7 +2739,7 @@ class BeamModulePlugin implements Plugin<Project> {
       // If none of them applied, version set here will be used as default 
value.
       // TODO(BEAM-12000): Move default value to Py3.9.

Review Comment:
   remove the todo



##########
sdks/python/test-suites/dataflow/common.gradle:
##########
@@ -351,6 +351,10 @@ task chicagoTaxiExample {
 
 task validatesContainer() {
   def pyversion = "${project.ext.pythonVersion.replace('.', '')}"
+  if (project.hasProperty("pipExtraOptions")) {

Review Comment:
   this is confusing when you read it outside of the context of this change.



##########
sdks/python/container/run_generate_requirements.sh:
##########
@@ -30,39 +30,46 @@
 # You will need Python interpreters for all versions supported by Beam, see:
 # https://s.apache.org/beam-python-dev-wiki
 
-if [[ $# != 2 ]]; then
+if [[ $# -lt 2 ]]; then
   printf "Example usage: \n$> 
./sdks/python/container/run_generate_requirements.sh 3.8 <sdk_tarball>"
-  printf "\n\twhere 3.8 is the Python major.minor version."
+  printf "\n\where 3.8 is the Python major.minor version."
   exit 1
 fi
 
 PY_VERSION=$1
 SDK_TARBALL=$2
+PIP_EXTRA_OPTIONS=$3
 
-if ! python$PY_VERSION --version > /dev/null 2>&1 ; then
+if ! python"$PY_VERSION" --version > /dev/null 2>&1 ; then
   echo "Please install a python${PY_VERSION} interpreter. See 
s.apache.org/beam-python-dev-wiki for Python installation tips."
   exit 1
 fi
 
-if ! python$PY_VERSION -m venv --help > /dev/null 2>&1 ; then
+if ! python"$PY_VERSION" -m venv --help > /dev/null 2>&1 ; then
   echo "Your python${PY_VERSION} installation does not have a required venv 
module. See s.apache.org/beam-python-dev-wiki for Python installation tips."
   exit 1
 fi
 
 set -ex
 
 ENV_PATH="$PWD/build/python${PY_VERSION/./}_requirements_gen"
-rm -rf $ENV_PATH 2>/dev/null || true
-python${PY_VERSION} -m venv $ENV_PATH
-source $ENV_PATH/bin/activate
+rm -rf "$ENV_PATH" 2>/dev/null || true
+python"${PY_VERSION}" -m venv "$ENV_PATH"
+source "$ENV_PATH"/bin/activate
 pip install --upgrade pip setuptools wheel
 
 # Install gcp extra deps since these deps are commonly used with Apache Beam.
 # Install dataframe deps to add have Dataframe support in released images.
 # Install test deps since some integration tests need dependencies,
 # such as pytest, installed in the runner environment.
-pip install --no-cache-dir $SDK_TARBALL[gcp,dataframe,test]
-pip install --no-cache-dir -r 
$PWD/sdks/python/container/base_image_requirements_manual.txt
+if [ -z "$PIP_EXTRA_OPTIONS" ]; then

Review Comment:
   you don't need the branch. default value is empty.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to