----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47296/#review132892 -----------------------------------------------------------
Thanks for contributing this patch. I believe that some more work needs to be done here before this patch gets approved. In particular, please see my comments on the VList.java changes. I believe this change will break the existing REST API usage in the Ambari Server integration layer, and so I'd recommend that at least that portion of the patch be reverted. Thanks. ambari-logsearch/ambari-logsearch-portal/src/main/java/org/apache/ambari/logsearch/view/VList.java <https://reviews.apache.org/r/47296/#comment197172> This change is problematic, since the Ambari Integration API already relies on the return results from the LogSearch Server. At this point in the dev cycle, we can't be changing the API at all from LogSearch, or the Ambari Integration will be broken. Because this is one of the types serialized in responses to the REST API, this is a part of the API, and at this point in the dev cycle should be frozen. I'd request that this part of the patch be reverted. Thanks. ambari-logsearch/ambari-logsearch-portal/src/main/java/org/apache/ambari/logsearch/web/security/LogsearchExternalServerAuthenticationProvider.java (line 53) <https://reviews.apache.org/r/47296/#comment197173> Looks like a type here, can you please fix this enum type to look like: "PRIVILEGE_INFO" Thanks ambari-logsearch/ambari-logsearch-portal/src/main/java/org/apache/ambari/logsearch/web/security/LogsearchExternalServerAuthenticationProvider.java (line 56) <https://reviews.apache.org/r/47296/#comment197174> Wouldn't it be simpler to just override the toString() methods for this enumerated type? - Robert Nettleton On May 12, 2016, 11:31 a.m., Dharmesh Makwana wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47296/ > ----------------------------------------------------------- > > (Updated May 12, 2016, 11:31 a.m.) > > > Review request for Ambari, Alejandro Fernandez, Andrew Onischuk, Don Bosco > Durai, Oliver Szabo, Robert Nettleton, Sandor Magyari, Sumit Mohanty, and > Sebastian Toader. > > > Bugs: AMBARI-16629 > https://issues.apache.org/jira/browse/AMBARI-16629 > > > Repository: ambari > > > Description > ------- > > Patch contains > 1) Authentication : change the API as per suggestion and support for roles in > authentication process. > 2) resultSize property removed from collection class. > 3) "Preview" issue fixed. > > > Diffs > ----- > > > ambari-logsearch/ambari-logsearch-portal/src/main/java/org/apache/ambari/logsearch/manager/LogsMgr.java > 0442cf9 > > ambari-logsearch/ambari-logsearch-portal/src/main/java/org/apache/ambari/logsearch/manager/UserConfigMgr.java > a60402e > > ambari-logsearch/ambari-logsearch-portal/src/main/java/org/apache/ambari/logsearch/query/QueryGenerationBase.java > cc61127 > > ambari-logsearch/ambari-logsearch-portal/src/main/java/org/apache/ambari/logsearch/util/JSONUtil.java > 8535039 > > ambari-logsearch/ambari-logsearch-portal/src/main/java/org/apache/ambari/logsearch/view/VList.java > 97226d2 > > ambari-logsearch/ambari-logsearch-portal/src/main/java/org/apache/ambari/logsearch/web/security/LogsearchExternalServerAuthenticationProvider.java > 79a414c > > ambari-logsearch/ambari-logsearch-portal/src/main/resources/logsearch.properties > 315d736 > > Diff: https://reviews.apache.org/r/47296/diff/ > > > Testing > ------- > > Tested in my local Ambari environment. > > > Thanks, > > Dharmesh Makwana > >
