> On Feb. 26, 2019, 12:56 p.m., Zsombor Gegesy wrote: > > embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/SolrSetupUtil.java > > Lines 204 (patched) > > <https://reviews.apache.org/r/70057/diff/2/?file=2126968#file2126968line204> > > > > I don't get, why do you want to return an empty list here - I guess, > > you can't do anything after this, the code will only report, that 'Core > > with name ... Already exists' - which is misleading. > > > > I would rather not catch these exceptions here.
In this case returning null. > On Feb. 26, 2019, 12:56 p.m., Zsombor Gegesy wrote: > > embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/SolrSetupUtil.java > > Lines 334 (patched) > > <https://reviews.apache.org/r/70057/diff/2/?file=2126968#file2126968line334> > > > > Am I right, that you just want to upload one file, which name match > > getConfigFileName()? > > > > If yes, then it's simpler to check if > > 'getConfigSetFolder()/getConfigFileName()' exists, and just upload that. I also need to check the content inside. > On Feb. 26, 2019, 12:56 p.m., Zsombor Gegesy wrote: > > embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/SolrSetupUtil.java > > Lines 415 (patched) > > <https://reviews.apache.org/r/70057/diff/2/?file=2126968#file2126968line415> > > > > Catching Throwable is not too good solution - if you really want to log > > problems here, please pick a more specific Exception, and rethrow it after > > the log. > > > > Or just remove this catch, and throw the exception to the caller as is > > - I mean, the original, specific exception I think its good to catch any exception here because it will let us know exactly on which operation exception occured. In this case like while "creating collection" > On Feb. 26, 2019, 12:56 p.m., Zsombor Gegesy wrote: > > embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/SolrSetupUtil.java > > Lines 510 (patched) > > <https://reviews.apache.org/r/70057/diff/2/?file=2126968#file2126968line510> > > > > Catching Exception is not too good solution - if you really want to log > > problems here, please pick a more specific exception, and rethrow it after > > the log. > > > > Or just remove this catch, and throw the exception to the caller as is > > - I mean, the original, specific exception I think its good to catch any exception here because it will let us know exactly on which operation exception occured. In this case like while "setting acl" > On Feb. 26, 2019, 12:56 p.m., Zsombor Gegesy wrote: > > embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/SolrSetupUtil.java > > Lines 531 (patched) > > <https://reviews.apache.org/r/70057/diff/2/?file=2126968#file2126968line531> > > > > I don't think, that you can do anything, if you can't talk to Solr, and > > you don't get the collections, I would rather have the exception propagated > > to the top. In this case returning null. - bhavik ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70057/#review213222 ----------------------------------------------------------- On March 14, 2019, 3:31 p.m., bhavik patel wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70057/ > ----------------------------------------------------------- > > (Updated March 14, 2019, 3:31 p.m.) > > > Review request for ranger, Ankita Sinha, Don Bosco Durai, Gautam Borad, Abhay > Kulkarni, Madhan Neethiraj, Oliver Szabo, Pradeep Agrawal, Ramesh Mani, > Selvamohan Neethiraj, Sailaja Polavarapu, and Velmurugan Periasamy. > > > Bugs: RANGER-2324 > https://issues.apache.org/jira/browse/RANGER-2324 > > > Repository: ranger > > > Description > ------- > > We are handling the solr bootstrapping in below mentioned manner for Ranger > 1.) Connection to solr > 2.) Upload Configuration > 3.) Create Collection > 4.) Setting ACL > > > Diffs > ----- > > > embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java > 8d32352 > > embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/SolrSetupUtil.java > PRE-CREATION > security-admin/scripts/install.properties fdcee1b > security-admin/scripts/setup.sh bd4bd4c > security-admin/src/main/resources/conf.dist/ranger-admin-default-site.xml > 686f720 > security-admin/src/main/resources/conf.dist/ranger-admin-site.xml 4d4a1de > src/main/assembly/admin-web.xml 8ea728b > > > Diff: https://reviews.apache.org/r/70057/diff/3/ > > > Testing > ------- > > Tested Below Scenario on ranger manual start / restart > 1.) Solr configuration were uploaded successfully > 2.) Solr collections were created successfully > 3.) ACL were setup as required. > > > Thanks, > > bhavik patel > >
