ksumit commented on code in PR #384:
URL: https://github.com/apache/incubator-livy/pull/384#discussion_r1089591529
##########
client-common/src/main/java/org/apache/livy/client/common/ClientConf.java:
##########
@@ -36,7 +36,7 @@
public abstract class ClientConf<T extends ClientConf>
implements Iterable<Map.Entry<String, String>> {
- protected Logger LOG = LoggerFactory.getLogger(getClass());
+ protected static Logger LOG = LoggerFactory.getLogger(ClientConf.class);
Review Comment:
should this be final too?
##########
server/src/main/scala/org/apache/livy/sessions/Session.scala:
##########
@@ -138,6 +138,7 @@ abstract class Session(
val id: Int,
val name: Option[String],
val owner: String,
+ val ttl: Option[String],
Review Comment:
should we add an auxiliary constructor as well that matches the existing
constructor (4 parameters only) and takes the global session timeout as ttl by
default? this will help reduce the amount of change in this PR.
##########
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 =
Review Comment:
nanoseconds seems too much to me, from my exposure to customer usecases i
think ttl in minutes should be enough. curious to know when nanoseconds level
of granularity would be useful.
please consider simplifying this further by making this an `Int` or `Long`
and avoiding all the parsing and validation logic. for the variable naming,
what do you think of `ttlInMinutes` or `ttlInSeconds`?
##########
docs/rest-api.md:
##########
@@ -151,6 +151,11 @@ Creates a new interactive Scala, Python, or R shell in the
cluster.
<td>Timeout in second to which session be orphaned</td>
<td>int</td>
</tr>
+ <tr>
+ <td>ttl</td>
+ <td>The timeout for this inactive session</td>
Review Comment:
would be useful to provide some examples here.
`timeout for this inactive session in minutes/seconds, example: 10`
--
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]
