Abacn commented on code in PR #34665:
URL: https://github.com/apache/beam/pull/34665#discussion_r2050647898


##########
sdks/python/apache_beam/yaml/yaml_provider.py:
##########
@@ -1184,10 +1184,10 @@ def _create_venv_from_scratch(
         subprocess.run([venv_pip, 'install'] + packages, check=True)
         with open(venv + '-requirements.txt', 'w') as fout:
           fout.write('\n'.join(packages))
-      except:  # pylint: disable=bare-except
+      except Exception as e:
+        logging.warning("Failed to create venv %s from scratch: %s", venv, e, 
exc_info=True)

Review Comment:
   _create_venv_from_scratch will return a non-existing path, and will fail 
elsewhere, even more difficult to debug if it happens.
   
   I think no more action is needed. The dependency is added after 34073



##########
sdks/python/apache_beam/yaml/yaml_provider.py:
##########
@@ -1184,10 +1184,10 @@ def _create_venv_from_scratch(
         subprocess.run([venv_pip, 'install'] + packages, check=True)
         with open(venv + '-requirements.txt', 'w') as fout:
           fout.write('\n'.join(packages))
-      except:  # pylint: disable=bare-except
+      except Exception as e:
+        logging.warning("Failed to create venv %s from scratch: %s", venv, e, 
exc_info=True)

Review Comment:
   _create_venv_from_scratch will return a non-existing path, and will fail 
elsewhere, even more difficult to debug if it happens.
   
   I think no more action is needed. The dependency is added after #34073



-- 
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-unsubscr...@beam.apache.org

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

Reply via email to