You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@livy.apache.org by "gyogal (via GitHub)" <gi...@apache.org> on 2023/01/25 17:04:48 UTC

[GitHub] [incubator-livy] gyogal commented on a diff in pull request #384: [LIVY-968][SERVER] Provide ttl field for a livy session

gyogal commented on code in PR #384:
URL: https://github.com/apache/incubator-livy/pull/384#discussion_r1086915353


##########
server/src/main/scala/org/apache/livy/sessions/SessionManager.scala:
##########
@@ -168,7 +169,9 @@ class SessionManager[S <: Session, R <: RecoveryMetadata : ClassTag](
             false
           } else {
             val currentTime = System.nanoTime()
-            currentTime - session.lastActivity > sessionTimeout
+            val calculatedTimeout =
+              ClientConf.getTimeAsNanos(session.ttl.orNull, session.id, sessionTimeout)

Review Comment:
   The TTL string is currently only validated here. If the formatting is invalid it seems like this will keep logging error messages every time `expired()` is called. To prevent this, the string could be validated as the session in created.



-- 
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: commits-unsubscribe@livy.apache.org

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