Explicit fail when python executable is not found #1236

aboellinger · 2022-02-28T15:58:52Z

Problem

We have found ourselves in a situation where during a call to find_pip_from_context, find_python_in_context(context) returns None, resulting in the creation and execution of an invalid command (with None provided as an executable path). This ends up breaking downstream in subprocess with an obscure message.

Solution

Raise an explicit error if the executable is not located, before we try to run it.

JeanChristopheMorinPerso · 2022-02-28T16:06:19Z

src/rez/pip.py

@@ -202,6 +202,9 @@ def find_pip_from_context(python_version, pip_version=None):

    py_exe = find_python_in_context(context)

+    if not py_exe:
+        raise RezSystemError("Failed to locate Python executable from context {}".format(context))


I think returning early like return None, None, context would be slightly better. That's because of the way the function is called: https://github.com/nerdvegas/rez/blob/master/src/rez/pip.py#L92-L101.

Oh, took me a while but I think I see what you mean.
This way we keep on with the logic of find_pip, hoping that we actually find a valid executable later.

I'll change that right away.

JeanChristopheMorinPerso · 2022-02-28T17:51:41Z

src/rez/pip.py

@@ -202,6 +202,10 @@ def find_pip_from_context(python_version, pip_version=None):

    py_exe = find_python_in_context(context)

+    if not py_exe:
+        print_debug("Failed to locate python executable from context")


This log might be redundant I think. The _check_found function already logs when something isn't found and I also have a personal preference to let the caller log instead of letting the callee (find_pip_from_context) log.

Alright, to me the log seemed consistent with code above and also acted as a comment for the return statement.

But I am fine with removing it.

Fixed in 2a64909