[GitHub] zeppelin issue #2039: [MINOR] Use standard java API to interrupt thread

2017-03-06 Thread felixcheung
Github user felixcheung commented on the issue: https://github.com/apache/zeppelin/pull/2039 merging if no more comment --- 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 an

[GitHub] zeppelin issue #2039: [MINOR] Use standard java API to interrupt thread

2017-03-06 Thread Leemoonsoo
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/2039 Code change Looks good to me. --- 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

[GitHub] zeppelin issue #2039: [MINOR] Use standard java API to interrupt thread

2017-03-06 Thread felixcheung
Github user felixcheung commented on the issue: https://github.com/apache/zeppelin/pull/2039 ok LGTM. any comment from other reviewers? --- 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 th

[GitHub] zeppelin issue #2039: [MINOR] Use standard java API to interrupt thread

2017-03-05 Thread felixcheung
Github user felixcheung commented on the issue: https://github.com/apache/zeppelin/pull/2039 Ok I wasn't aware of the thread.interrupt semantic in JVM. --- 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

[GitHub] zeppelin issue #2039: [MINOR] Use standard java API to interrupt thread

2017-03-02 Thread DrIgor
Github user DrIgor commented on the issue: https://github.com/apache/zeppelin/pull/2039 `interrupt` just sets `Thread.isInterrupted` flag and does not immediately stop the thread. Then thread can check this flag and decide if it needs to stop. So in fact, termination logic remains the

[GitHub] zeppelin issue #2039: [MINOR] Use standard java API to interrupt thread

2017-03-02 Thread felixcheung
Github user felixcheung commented on the issue: https://github.com/apache/zeppelin/pull/2039 From my understanding, setting a flag to let the thread gracefully runs to end, seems safer than calling `interrupt` on it --- If your project is set up for it, you can reply to this email an

[GitHub] zeppelin issue #2039: [MINOR] Use standard java API to interrupt thread

2017-03-02 Thread DrIgor
Github user DrIgor commented on the issue: https://github.com/apache/zeppelin/pull/2039 Ping-ping) Do you have other questions? Do you think it's better to proper synchronize `terminate` flag instead? I reviewed code to understand how zeppelin core works and found sev

[GitHub] zeppelin issue #2039: [MINOR] Use standard java API to interrupt thread

2017-02-20 Thread DrIgor
Github user DrIgor commented on the issue: https://github.com/apache/zeppelin/pull/2039 @felixcheung `terminate` flag is shared across several threads and is accessed without any synchronization. And we don't need to implement custom interruption flag because java already has it รข

[GitHub] zeppelin issue #2039: [MINOR] Use standard java API to interrupt thread

2017-02-20 Thread felixcheung
Github user felixcheung commented on the issue: https://github.com/apache/zeppelin/pull/2039 for a change like this, it would be useful to open a JIRA and to discuss why we are making this change --- If your project is set up for it, you can reply to this email and have your reply ap