----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64404/#review193102 -----------------------------------------------------------
I think there is a regression here by removing the "break;" if the response is 200. The old code broke out of the for loop on the first "200" response, but the new code will loop through all of the for loop and just use the last "Response" in the next section. - Colm O hEigeartaigh On Dec. 7, 2017, 5:54 a.m., pengjianhua wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64404/ > ----------------------------------------------------------- > > (Updated Dec. 7, 2017, 5:54 a.m.) > > > Review request for ranger, Alok Lal, Ankita Sinha, Don Bosco Durai, Colm O > hEigeartaigh, Gautam Borad, Madhan Neethiraj, Ramesh Mani, Selvamohan > Neethiraj, Velmurugan Periasamy, and Qiang Zhang. > > > Bugs: RANGER-1918 > https://issues.apache.org/jira/browse/RANGER-1918 > > > Repository: ranger > > > Description > ------- > > 1.We have already determined the logic of the response! = Null and > response.getStatus ()! = 200 in the getQueueResponse method, and do not need > to be evaluated in the run method of the getQueueResponse method. Move > "response.close ();" into the getQueueResponse method. > 2.There is no necessary to create so many useless temporary variables "String > errMsg = errMessage;" > just use errMessage is better practice. > > > Diffs > ----- > > > plugin-yarn/src/main/java/org/apache/ranger/services/yarn/client/YarnClient.java > b61a07e > > > Diff: https://reviews.apache.org/r/64404/diff/1/ > > > Testing > ------- > > Tested it. > > > Thanks, > > pengjianhua > >
