[GitHub] storm pull request: [STORM-442] multilang ShellBolt/ShellSpout die...

2015-01-06 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/305 --- 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

[GitHub] storm pull request: [STORM-442] multilang ShellBolt/ShellSpout die...

2015-01-06 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/305#issuecomment-68936183 No other comments in the past two weeks so I am going to check this in. --- If your project is set up for it, you can reply to this email and have your reply appear on

[GitHub] storm pull request: [STORM-442] multilang ShellBolt/ShellSpout die...

2014-12-23 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/305#issuecomment-67989780 I see this has not had that much traffic in quite a while. The code to me looks acceptable. I am not really happy with calling .available() twice, but I can live with

[GitHub] storm pull request: [STORM-442] multilang ShellBolt/ShellSpout die...

2014-11-05 Thread dashengju
Github user dashengju commented on the pull request: https://github.com/apache/storm/pull/305#issuecomment-61774651 @clockfly I think it is not a good idea to kill sub process. Because when we catch a exception, we do not know whether sub process is still running. kill pid may

[GitHub] storm pull request: [STORM-442] multilang ShellBolt/ShellSpout die...

2014-11-04 Thread itaifrenkel
Github user itaifrenkel commented on the pull request: https://github.com/apache/storm/pull/305#issuecomment-61631559 @clockfly - Please clarify. When you say the parent process fails do you mean that the worker process is no longer running? If this is the case that process cannot

[GitHub] storm pull request: [STORM-442] multilang ShellBolt/ShellSpout die...

2014-11-04 Thread clockfly
Github user clockfly commented on the pull request: https://github.com/apache/storm/pull/305#issuecomment-61631943 @itaifrenkel, ``` When you say the parent process fails do you mean that the worker process is no longer running? ``` No, it means the case

[GitHub] storm pull request: [STORM-442] multilang ShellBolt/ShellSpout die...

2014-11-04 Thread HeartSaVioR
Github user HeartSaVioR commented on a diff in the pull request: https://github.com/apache/storm/pull/305#discussion_r19839876 --- Diff: storm-core/src/jvm/backtype/storm/utils/ShellProcess.java --- @@ -135,7 +135,14 @@ public void logErrorStream() { public String

[GitHub] storm pull request: [STORM-442] multilang ShellBolt/ShellSpout die...

2014-11-02 Thread dashengju
Github user dashengju commented on the pull request: https://github.com/apache/storm/pull/305#issuecomment-61447792 @HeartSaVioR yes, there is a chance to occur that subprocess writes stderr for some reason but parent process runs well without Exception. When this happens,

[GitHub] storm pull request: [STORM-442] multilang ShellBolt/ShellSpout die...

2014-10-31 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request: https://github.com/apache/storm/pull/305#issuecomment-61269957 If we're narrowing issue to just handle parent process's Exception hang, looks good to me. (We can make it better with extracting logic that reads from stderr to

[GitHub] storm pull request: [STORM-442] multilang ShellBolt/ShellSpout die...

2014-10-29 Thread dashengju
Github user dashengju commented on the pull request: https://github.com/apache/storm/pull/218#issuecomment-61038451 @HeartSaVioR , parent process's exception contains two types: 1) net io error caused; for example, subprocess report an error to stderr and exit; 2) parent