> On May 16, 2017, 1:34 a.m., Alexander Kolbasov wrote: > > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPermissions.java > > Lines 73 (patched) > > <https://reviews.apache.org/r/59120/diff/5/?file=1716362#file1716362line73> > > > > What is this change and how it relates to the issue?
We can remove this output. I made that change when I was debugging why the test failed. > On May 16, 2017, 1:34 a.m., Alexander Kolbasov wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > > Lines 135 (patched) > > <https://reviews.apache.org/r/59120/diff/5/?file=1716363#file1716363line143> > > > > The comment says "therefore return null here" but the code throws an > > exception instead. Do we really want to throw such exception? I will change it to return null instead of throwing exception. > On May 16, 2017, 1:34 a.m., Alexander Kolbasov wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > > Lines 140 (patched) > > <https://reviews.apache.org/r/59120/diff/5/?file=1716363#file1716363line148> > > > > in other places you call it metastore (one word), let's be consistent > > here and call it metastore as well. > > > > Also, I think it is sueful to log metastore URI in general, so why not > > make it an info log and say something like > > > > "Connecting to Hive metastore using URI {}" will update > On May 16, 2017, 1:34 a.m., Alexander Kolbasov wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > > Lines 201 (patched) > > <https://reviews.apache.org/r/59120/diff/5/?file=1716363#file1716363line209> > > > > We already have it - why do we need to read it again? will reuse it. And remove this line > On May 16, 2017, 1:34 a.m., Alexander Kolbasov wrote: > > sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java > > Line 19 (original), 19 (patched) > > <https://reviews.apache.org/r/59120/diff/5/?file=1716364#file1716364line19> > > > > For some reason all these imports are showing up as unused - do you > > know why? It does not show up as unused for me. I guess it is because by default the hive-v2 tests are not activated in sentry-tests/pom.xml <profile> <id>hive-authz2</id> <activation> <activeByDefault>false</activeByDefault> </activation> <modules> <module>sentry-tests-hive-v2</module> </modules> </profile> - Na ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59120/#review175051 ----------------------------------------------------------- On May 12, 2017, 3:40 a.m., Na Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59120/ > ----------------------------------------------------------- > > (Updated May 12, 2017, 3:40 a.m.) > > > Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and > Sergio Pena. > > > Bugs: sentry-1757 > https://issues.apache.org/jira/browse/sentry-1757 > > > Repository: sentry > > > Description > ------- > > use hive configuration to check. > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPermissions.java > 431c7fe > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > 9880c40 > > sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java > 1530eb2 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java > 7d128b7 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java > 32acc65 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java > 548dcf8 > > > Diff: https://reviews.apache.org/r/59120/diff/5/ > > > Testing > ------- > > integration test TestHDFSIntegrationEnd2End > > > Thanks, > > Na Li > >
