jahstreet commented on code in PR #462:
URL: https://github.com/apache/incubator-livy/pull/462#discussion_r1927658781


##########
server/src/main/scala/org/apache/livy/utils/SparkKubernetesApp.scala:
##########
@@ -138,6 +138,7 @@ object SparkKubernetesApp extends Logging {
   private var sessionLeakageCheckInterval: Long = _
 
   var kubernetesClient: DefaultKubernetesClient = _
+  var appNamespaces: mutable.Set[String] = mutable.Set("default")

Review Comment:
   Why do we need to have a default namespaces at all? Can it be empty 
initially? Also is it thread safe (or can we make it so)?



##########
server/src/main/scala/org/apache/livy/utils/SparkKubernetesApp.scala:
##########
@@ -138,6 +138,7 @@ object SparkKubernetesApp extends Logging {
   private var sessionLeakageCheckInterval: Long = _
 
   var kubernetesClient: DefaultKubernetesClient = _
+  var appNamespaces: mutable.Set[String] = mutable.Set("default")

Review Comment:
   Did you consider making configurable allowed namespaces list? We anyway know 
it upfront when configuring permissions for Live service account. It worths 
checking how the recovery works, I haven't looked at it for a while, but having 
Livy monitoring all the namespaces from the get go may save us from 
leaked/unrecovered apps.



##########
server/src/main/scala/org/apache/livy/utils/SparkKubernetesApp.scala:
##########
@@ -138,6 +138,7 @@ object SparkKubernetesApp extends Logging {
   private var sessionLeakageCheckInterval: Long = _
 
   var kubernetesClient: DefaultKubernetesClient = _
+  var appNamespaces: mutable.Set[String] = mutable.Set("default")

Review Comment:
   Then we also do not need to propagate app namespace everywhere.
   WDYT?



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