[GitHub] [beam] ihji commented on a diff in pull request #22462: Upgrades pip before installing Beam for Python default expansion service

[GitBox <gi...@apache.org>]

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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org