STORM-414: support logging level to multilang protocol spout and bolt #196
Conversation
dashengju
changed the title
| @@ -158,6 +157,33 @@ private void querySubprocess() { | |||
| } | |||
| } | |||
|
|
|||
| private void handleLog(ShellMsg shellMsg) { | |||
| String msg = shellMsg.getMsg(); | |||
| msg = "ShellLog" + _process.getProcessInfoString() + msg; | |||
There was a problem hiding this comment.
Could we have some white space in here? to split some of this up.
There was a problem hiding this comment.
I have noticed the white space. Because _process.getProcessInfoString() return a string with white space at begin and end, so I did not add white space here.
And I think the best way is remove white space in _process.getProcessInfoString()'s return, and add white space in here explicitly, and I have change the code.
|
The code looks great as is. I had a few minor comments though that should hopefully make this feature easier to use. |
|
I have modified the white space problem and add more log function for easy use. thanks for your review and suggestion, and please review again. |
|
Thanks the changes look good to me I am +1 for this. I'll try to check it in tomorrow so others can have some more time to look at it and comment if they want to. |
|
+1 |
|
@dashengju d2r is going to be out until at least Tuesday. You have 2 +1s already and I should have kept my promise to check it in earlier so I am just going to merge it in. |
|
thanks :) |
[BUG 6934347] Added some robustness to auto-creds with no credentials.
https://issues.apache.org/jira/browse/STORM-414
@mahall create a PR: #24 , Added logging level to multilang protocol spout and bolt. But he closed it with no reason.
@msukmanowsky create a PR before to apache: nathanmarz/storm#626 , Allow ShellBolts to optionally specify the logging level. But he did not modify ShellSpout and storm.py.
This improvement add optional logging level to Multilang Protocol's log method. And add implementation in python and ruby.