ihji commented on code in PR #22462:
URL: https://github.com/apache/beam/pull/22462#discussion_r931351666


##########
sdks/java/extensions/python/src/main/resources/org/apache/beam/sdk/extensions/python/bootstrap_beam_venv.py:
##########
@@ -92,6 +92,17 @@ def maybe_strict_version(s):
     if not os.path.exists(venv_python):
         try:
             subprocess.run([executable, '-m', 'venv', venv_dir], check=True)
+
+            # Upgrading pip and setuptools for the virtual environment.
+            subprocess.run([
+                venv_python, '-m', 'pip', 'install', '--upgrade', 'pip'

Review Comment:
   Would this two additional installations increase the launch time? Was there 
any noticeable difference?
   
   



##########
sdks/java/extensions/python/src/main/resources/org/apache/beam/sdk/extensions/python/bootstrap_beam_venv.py:
##########
@@ -92,6 +92,17 @@ def maybe_strict_version(s):
     if not os.path.exists(venv_python):
         try:
             subprocess.run([executable, '-m', 'venv', venv_dir], check=True)
+
+            # Upgrading pip and setuptools for the virtual environment.

Review Comment:
   Looks good, thanks. My only concern is that automatic pip update to the 
latest version may also break working codes in the future since flexible 
dependency allows external dependency updates could affect the behavior of 
existing codes with no change.
   
   Did you consider tradeoffs between upgrading to the latest vs. specifying a 
fixed working version? 



-- 
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