> On 三月 21, 2015, 4:58 p.m., Prasad Mujumdar wrote: > > sentry-policy/sentry-policy-sqoop/pom.xml, line 83 > > <https://reviews.apache.org/r/31617/diff/2/?file=898531#file898531line83> > > > > Nit: Do we need to specify version here ?
hi prasad. You are right. It isn't required. I will fix it. Thanks for your suggest. > On 三月 21, 2015, 4:58 p.m., Prasad Mujumdar wrote: > > sentry-policy/sentry-policy-sqoop/src/test/java/org/apache/sentry/policy/sqoop/TestServerNameRequiredMatch.java, > > line 44 > > <https://reviews.apache.org/r/31617/diff/2/?file=898538#file898538line44> > > > > It should check that the exception is actually authorization exception > > so that other unexpected errors won't go unnoticed. I agree with your comment. I will fix it. > On 三月 21, 2015, 4:58 p.m., Prasad Mujumdar wrote: > > sentry-policy/sentry-policy-sqoop/src/test/java/org/apache/sentry/policy/sqoop/TestSqoopAuthorizationProviderGeneralCases.java, > > lines 83-85 > > <https://reviews.apache.org/r/31617/diff/2/?file=898539#file898539line83> > > > > Nit: It would be nice if these are defined as constants. Ok, I will fix it. Thanks > On 三月 21, 2015, 4:58 p.m., Prasad Mujumdar wrote: > > sentry-policy/sentry-policy-sqoop/src/test/java/org/apache/sentry/policy/sqoop/TestSqoopAuthorizationProviderGeneralCases.java, > > line 91 > > <https://reviews.apache.org/r/31617/diff/2/?file=898539#file898539line91> > > > > It would be good add some negative scenarios for analyst and developer > > roles. Ok, Thanks for your comment. I will fix it. > On 三月 21, 2015, 4:58 p.m., Prasad Mujumdar wrote: > > sentry-policy/sentry-policy-sqoop/src/test/java/org/apache/sentry/policy/sqoop/TestSqoopPolicyEngineDFS.java, > > line 34 > > <https://reviews.apache.org/r/31617/diff/2/?file=898542#file898542line34> > > > > Do we need this to be a new class in sqoop package ? If this is direct > > copy/paste from other modules, would be it be possible to move these common > > helper classes to a new test-common module ? I will have a try about your advice. Thanks - shen ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31617/#review77334 ----------------------------------------------------------- On 三月 18, 2015, 2:02 a.m., shen guoquan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31617/ > ----------------------------------------------------------- > > (Updated 三月 18, 2015, 2:02 a.m.) > > > Review request for sentry, Sqoop, Abraham Elmahrek, Xiaomeng Huang, Colin Ma, > Dapeng Sun, and Prasad Mujumdar. > > > Repository: sentry > > > Description > ------- > > Add Sqoop policy engine in Sentry for Sqoop role-based authorizaiton > > > Diffs > ----- > > pom.xml 4c80916 > sentry-dist/pom.xml f63b33b > sentry-policy/pom.xml 4fb4f3c > sentry-policy/sentry-policy-sqoop/pom.xml PRE-CREATION > > sentry-policy/sentry-policy-sqoop/src/main/java/org/apache/sentry/policy/sqoop/ServerNameRequiredMatch.java > PRE-CREATION > > sentry-policy/sentry-policy-sqoop/src/main/java/org/apache/sentry/policy/sqoop/SimpleSqoopPolicyEngine.java > PRE-CREATION > > sentry-policy/sentry-policy-sqoop/src/main/java/org/apache/sentry/policy/sqoop/SqoopModelAuthorizables.java > PRE-CREATION > > sentry-policy/sentry-policy-sqoop/src/main/java/org/apache/sentry/policy/sqoop/SqoopWildcardPrivilege.java > PRE-CREATION > > sentry-policy/sentry-policy-sqoop/src/test/java/org/apache/sentry/policy/sqoop/AbstractTestSqoopPolicyEngine.java > PRE-CREATION > > sentry-policy/sentry-policy-sqoop/src/test/java/org/apache/sentry/policy/sqoop/SqoopPolicyFileProviderBackend.java > PRE-CREATION > > sentry-policy/sentry-policy-sqoop/src/test/java/org/apache/sentry/policy/sqoop/TestServerNameRequiredMatch.java > PRE-CREATION > > sentry-policy/sentry-policy-sqoop/src/test/java/org/apache/sentry/policy/sqoop/TestSqoopAuthorizationProviderGeneralCases.java > PRE-CREATION > > sentry-policy/sentry-policy-sqoop/src/test/java/org/apache/sentry/policy/sqoop/TestSqoopAuthorizationProviderSpecialCases.java > PRE-CREATION > > sentry-policy/sentry-policy-sqoop/src/test/java/org/apache/sentry/policy/sqoop/TestSqoopModelAuthorizables.java > PRE-CREATION > > sentry-policy/sentry-policy-sqoop/src/test/java/org/apache/sentry/policy/sqoop/TestSqoopPolicyEngineDFS.java > PRE-CREATION > > sentry-policy/sentry-policy-sqoop/src/test/java/org/apache/sentry/policy/sqoop/TestSqoopPolicyEngineLocalFS.java > PRE-CREATION > > sentry-policy/sentry-policy-sqoop/src/test/java/org/apache/sentry/policy/sqoop/TestSqoopWildcardPrivilege.java > PRE-CREATION > sentry-policy/sentry-policy-sqoop/src/test/resources/log4j.properties > PRE-CREATION > > sentry-policy/sentry-policy-sqoop/src/test/resources/test-authz-provider.ini > PRE-CREATION > > Diff: https://reviews.apache.org/r/31617/diff/ > > > Testing > ------- > > Ran the unit tests. > > > Thanks, > > shen guoquan > >
