Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/13599#discussion_r163194199
--- Diff:
core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -82,6 +90,12 @@ private[spark] class PythonWorkerFactory(pythonExec:
String, envVars: Map[String
envVars.getOrElse("PYTHONPATH", ""),
sys.env.getOrElse("PYTHONPATH", ""))
+
+ if (virtualEnvEnabled) {
+ val virtualEnvFactory = new VirtualEnvFactory(pythonExec, conf, false)
+ virtualenvPythonExec = Some(virtualEnvFactory.setupVirtualEnv())
+ }
--- End diff --
I guess we don't prefer unnecessary `var`s.
How about the following with the diff above:
```scala
val virtualEnvEnabled = conf.getBoolean("spark.pyspark.virtualenv.enabled",
false)
val virtualenvPythonExec = if (virtualEnvEnabled) {
val virtualEnvFactory = new VirtualEnvFactory(pythonExec, conf, false)
Some(virtualEnvFactory.setupVirtualEnv())
} else {
None
}
```
Or maybe we can:
```scala
val virtualEnvEnabled = conf.getBoolean("spark.pyspark.virtualenv.enabled",
false)
val virtualenvPythonExec = if (virtualEnvEnabled) {
val virtualEnvFactory = new VirtualEnvFactory(pythonExec, conf, false)
virtualEnvFactory.setupVirtualEnv()
} else {
pythonExec
}
```
and use this directly.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
