[GitHub] [incubator-livy] yantzu commented on a change in pull request #182: [LIVY-571] cleanupStatement should not throw exception when statementId not exist

[GitBox <gi...@apache.org>]

yantzu commented on a change in pull request #182: [LIVY-571] cleanupStatement should not throw exception when statementId not exist
URL: https://github.com/apache/incubator-livy/pull/182#discussion_r304708848
 
 

 ##########
 File path: thriftserver/session/src/main/java/org/apache/livy/thriftserver/session/ThriftSessionState.java
 ##########
 @@ -104,9 +109,10 @@ StatementState findStatement(String statementId) {
   void cleanupStatement(String statementId) {
     checkNotNull(statementId, "No statement ID.");
     if (statements.remove(statementId) == null) {
-      throw statementNotFound(statementId);
+      LOG.warn("Statement {} not found in session {}",statementId, sessionId);
 
 Review comment:
   I did consider other solution like catch exception in livy or don't CleanupStatementJob to spark when livy know it is a ERROR statement. But I think it make sense to change cleanupStatement, it like remove method of Map, or close method of InputStream, not matter how many times it is invoked, should always return success.
   
   Other comments are fixed, and thanks a lot for your great suggestion!

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services