----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55008/#review160334 -----------------------------------------------------------
embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java (lines 312 - 313) <https://reviews.apache.org/r/55008/#comment231489> I agree with Alok, blocks without braces are like disastor waiting to happen. It would be good to enclose within brackets embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java (line 315) <https://reviews.apache.org/r/55008/#comment231490> For utility methods, I generally prefer to provide the exception as the second param to the logger. This provides the stack trace to identify who made the call with the bad values. Similar feedback for the references in getLongConfig embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java (line 433) <https://reviews.apache.org/r/55008/#comment231491> Same feedback as before, better to pass the exception as parameter, than just printing the message. - Don Bosco Durai On Dec. 23, 2016, 6:54 a.m., Qiang Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55008/ > ----------------------------------------------------------- > > (Updated Dec. 23, 2016, 6:54 a.m.) > > > Review request for ranger, Don Bosco Durai, Madhan Reddy, Ramesh Mani, > Selvamohan Neethiraj, and Velmurugan Periasamy. > > > Bugs: RANGER-1280 > https://issues.apache.org/jira/browse/RANGER-1280 > > > Repository: ranger > > > Description > ------- > > I failed to run the ranger-admin command and couldn't find any error messages > in log file after set > ranger.service.http.connector.attrib.maxHeaderCount=aa(string value) in > ranger-admin-site.xml file. I analyzed the source codes and found the reason. > I found there are two questions after check the source codes in > EmbeddedServer.java. > 1. No exception pop out when called Integer.parseInt() and Long.parseLong() > function. Once abnormal, the program aborted directly without log. > 2. The catch captures anomaly without log in the loadConfig function. It only > calls e.printStackTrace() function. > > Currently the program will be aborted when the exception occured. We should > get the default value instead of aborted. > > We have tested our patch strictly. > > > Diffs > ----- > > > embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java > 7ebba8a > > Diff: https://reviews.apache.org/r/55008/diff/ > > > Testing > ------- > > > Thanks, > > Qiang Zhang > >
