> On Nov. 8, 2013, 3:19 p.m., Brock Noland wrote: > > sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java, > > line 117 > > <https://reviews.apache.org/r/15335/diff/1/?file=380751#file380751line117> > > > > Are Solr exceptions that good that we don't need to keep the stack > > trace? > > Gregory Chanan wrote: > Good question. My thought was: > 1) all the SolrExceptions in the current implementation are thrown > directly in the HdfsUtil.addHdfsResources function, so we are only losing the > top-level element of the stack > 2) the SolrExceptions thrown all have "SERVER_ERROR" in the message, > which felt confusing to me, because there is nothing wrong with the solr > server proper, only with sentry. > > But I could see arguments the other way. Happy to do what you think is > best, Brock. What do you think?
Unless there is a really good reason I hate losing stack traces. A year down the road the implementations of that method might be totally different and we could potentially totally impact someone's weekend over not nesting. It's happened to me more times than I can count! :) - Brock ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15335/#review28544 ----------------------------------------------------------- On Nov. 8, 2013, 2 a.m., Gregory Chanan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15335/ > ----------------------------------------------------------- > > (Updated Nov. 8, 2013, 2 a.m.) > > > Review request for sentry, Brock Noland and Shreepadma Venugopalan. > > > Repository: sentry > > > Description > ------- > > Today, the SolrAuthzBinding uses the SimpleFileProviderBackend, which ends up > creating a configuration via: > > new Configuration(); > > the issue with this is this might not be the Configuration we want – for > example, in a secure setup, it may not have all the > authentication/authorization properties necessary to connect to hdfs to read > the sentry .ini file. I'm not sure exactly how this works in hive, but it may > be because hive has the relevant core-site.xml, hdfs-site.xml in its > classpath, whereas in solr, this is uncommon. > > Solr does have a way to specify hdfs configs already via the > HdfsDirectoryFactory and HdfsUtils -- I just reuse this and add a constructor > to SimpleFileProviderBackend. > > > Diffs > ----- > > pom.xml 997b1f4 > sentry-binding/sentry-binding-solr/pom.xml 4ed491a > > sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java > 94f9c57 > > sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java > 5e0aa66 > > Diff: https://reviews.apache.org/r/15335/diff/ > > > Testing > ------- > > Ran the non-e2e unit tests. > > Also ran the code with a real solr cluster to verify that the correct > Configuration was loaded and we could read a .ini file from secure HDFS. > > > Thanks, > > Gregory Chanan > >
