exceptionfactory commented on code in PR #8961:
URL: https://github.com/apache/nifi/pull/8961#discussion_r1640384193


##########
nifi-extension-bundles/nifi-py4j-bundle/nifi-py4j-bridge/src/main/java/org/apache/nifi/py4j/PythonProcess.java:
##########
@@ -302,8 +302,14 @@ String resolvePythonCommand() throws IOException {
         } else if (virtualEnvDirectories.length == 1) {
             commandExecutableDirectory = virtualEnvDirectories[0].getName();
         } else {
-            // Default to bin directory for macOS and Linux
-            commandExecutableDirectory = "bin";
+            // We should get at most 2 directories.  Check for python command.
+            if (virtualEnvDirectories[0].list((file, name) -> 
name.startsWith(pythonCmd)).length >= 1) {

Review Comment:
   The `list()` method can return `null` under some circumstances, so adjusting 
the approach seems necessary. What about moving this block into a separate 
method named something like `findExecutableDirectory`? That method could 
iterate over the `virtualEnvDirectories` array, and not necessary assume that 
there will be at most two directories.



##########
nifi-extension-bundles/nifi-py4j-bundle/nifi-py4j-bridge/src/main/java/org/apache/nifi/py4j/PythonProcess.java:
##########
@@ -302,8 +302,14 @@ String resolvePythonCommand() throws IOException {
         } else if (virtualEnvDirectories.length == 1) {
             commandExecutableDirectory = virtualEnvDirectories[0].getName();
         } else {
-            // Default to bin directory for macOS and Linux
-            commandExecutableDirectory = "bin";
+            // We should get at most 2 directories.  Check for python command.
+            if (virtualEnvDirectories[0].list((file, name) -> 
name.startsWith(pythonCmd)).length >= 1) {
+                commandExecutableDirectory = 
virtualEnvDirectories[0].getName();
+            } else if (virtualEnvDirectories[1].list((file, name) -> 
name.startsWith(pythonCmd)).length >= 1) {
+                commandExecutableDirectory = 
virtualEnvDirectories[1].getName();
+            } else {
+                throw new IOException("Failed to find pythond command: " + 
pythonCmd);

Review Comment:
   Minor spelling and string construction recommendation:
   ```suggestion
                   throw new IOException("Failed to find Python command 
[%s]".formatted(pythonCmd));
   ```



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