[GitHub] zeppelin issue #1144: [ZEPPELIN-1128] add try-catch in close() method.

2016-07-18 Thread jongyoul
Github user jongyoul commented on the issue:

https://github.com/apache/zeppelin/pull/1144
  
@voyageth No problem. I like defensive programming. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zeppelin issue #1144: [ZEPPELIN-1128] add try-catch in close() method.

2016-07-18 Thread jongyoul
Github user jongyoul commented on the issue:

https://github.com/apache/zeppelin/pull/1144
  
LGTM. Merging if there's no more discussion.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zeppelin issue #1144: [ZEPPELIN-1128] add try-catch in close() method.

2016-07-18 Thread voyageth
Github user voyageth commented on the issue:

https://github.com/apache/zeppelin/pull/1144
  
@jongyoul I change this just for more stability. I can't find that bug case 
just now. But if some Connection or Statement's close() method has some 
bug(like NPE is some circumstance), zeppelin can not clear that resource.

But if you think Exception is too much, I'll change SqlException.
Thanks for feedback.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zeppelin issue #1144: [ZEPPELIN-1128] add try-catch in close() method.

2016-07-18 Thread jongyoul
Github user jongyoul commented on the issue:

https://github.com/apache/zeppelin/pull/1144
  
@voyageth Where have you checked those exceptions? Isn't SqlException 
enough?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zeppelin issue #1144: [ZEPPELIN-1128] add try-catch in close() method.

2016-07-12 Thread voyageth
Github user voyageth commented on the issue:

https://github.com/apache/zeppelin/pull/1144
  
Change catch SQLException to Exception. I think connection.close() can 
throw other Exception.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---