> On Jan. 2, 2017, 5:58 p.m., Don Bosco Durai wrote:
> > embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java,
> > lines 312-314
> > <https://reviews.apache.org/r/55008/diff/1/?file=1591529#file1591529line312>
> >
> > I agree with Alok, blocks without braces are like disastor waiting to
> > happen. It would be good to enclose within brackets
At the beginning of the getIntConfig I have set a default value for the ret.
int ret = defaultValue;
The following logic can be executed when the retStr is null.
else
ret = defaultValue;
Since we have default the value for ret, so there is no need to execute the
logic.
So we don't need to enclose within brackets.
> On Jan. 2, 2017, 5:58 p.m., Don Bosco Durai wrote:
> > embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java,
> > line 316
> > <https://reviews.apache.org/r/55008/diff/1/?file=1591529#file1591529line316>
> >
> > 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
Ok. Thanks. We will update the error message as below:
LOG.warning(retStr + " can't be parsed to int. Reason: " + err.toString());
> On Jan. 2, 2017, 5:58 p.m., Don Bosco Durai wrote:
> > embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java,
> > line 434
> > <https://reviews.apache.org/r/55008/diff/1/?file=1591529#file1591529line434>
> >
> > Same feedback as before, better to pass the exception as parameter,
> > than just printing the message.
Ok. Thanks. We will update the error message as below:
LOG.warning("Load configuration fail. Reason: " + e.toString() );
- Qiang
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55008/#review160334
-----------------------------------------------------------
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
>
>
