> On Feb. 29, 2016, 9:38 a.m., Dapeng Sun wrote:
> > Thank Ashish, I left some comments.

Dapeng thanks for the review. Addressed your concerns.


> On Feb. 29, 2016, 9:38 a.m., Dapeng Sun wrote:
> > sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/KafkaTestServer.java,
> >  line 68
> > <https://reviews.apache.org/r/43977/diff/1/?file=1270528#file1270528line68>
> >
> >     need sync?

Good point. Fixed.


> On Feb. 29, 2016, 9:38 a.m., Dapeng Sun wrote:
> > sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAclsCrud.java,
> >  line 44
> > <https://reviews.apache.org/r/43977/diff/1/?file=1270532#file1270532line44>
> >
> >     Is sentryKafkaAuthorizer depended on AbstractKafkaSentryTestBase? If 
> > not, we should move the tests to "SENTRY-1057 Add implementations for acls' 
> > CRUD"

Makes sense/ moving.


- Ashish


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43977/#review121214
-----------------------------------------------------------


On Feb. 25, 2016, 1:53 a.m., Ashish Singh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43977/
> -----------------------------------------------------------
> 
> (Updated Feb. 25, 2016, 1:53 a.m.)
> 
> 
> Review request for sentry, Dapeng Sun and Hao Hao.
> 
> 
> Bugs: SENTRY-1014
>     https://issues.apache.org/jira/browse/SENTRY-1014
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1014: Add E2E tests for Kafka plugin.
> 
> 
> Diffs
> -----
> 
>   pom.xml ca2c92a26e3e42fe036c974235db47255d1465de 
>   sentry-binding/sentry-binding-kafka/pom.xml 
> bd24c20edcbffb5caee69aedddb93cb5a6c8c112 
>   
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/authorizer/SentryKafkaAuthorizer.java
>  9ffb971d8da51b716ece3ff5a5bbeebcc7f74e8d 
>   
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java
>  9e72d7890e0db3710ab0971ccbb71a32d3d36e87 
>   
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBindingSingleton.java
>  92e50e6452a89e676eb559a3657b17ba563bfddc 
>   
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/conf/KafkaAuthConf.java
>  e75ec7eddaa1a48b731d551736859a94c08af593 
>   
> sentry-binding/sentry-binding-kafka/src/test/java/org/apache/sentry/kafka/authorizer/SentryKafkaAuthorizerTest.java
>  eafe0f0ee5482fadee43ddec99bdc5da3f42e30f 
>   
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Cluster.java
>  bb30b1b78d4dccaf8ea5fe22eeed496b608cfcb1 
>   
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaActionFactory.java
>  7b8b5187e3d95a49304512e238e778da885fd27d 
>   
> sentry-core/sentry-core-model-kafka/src/test/java/org/apache/sentry/core/model/kafka/TestKafkaAuthorizable.java
>  20d5e8e518cb91b32fad930e9d39400764f40b55 
>   
> sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaModelAuthorizables.java
>  f1ed0001883e5d4b362389c258f4f72697a1e5a4 
>   
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaModelAuthorizables.java
>  513c2719424f994e8eb773e72073401b465b7d58 
>   
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/provider/TestKafkaAuthorizationProviderGeneralCases.java
>  bcc119860860d928d092ed3ddc0a5a28e841c9aa 
>   sentry-provider/sentry-provider-db/pom.xml 
> 7514a7cdfcc7934f2dd0386996fdaf88c0ccbb14 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java
>  c3b0be8694c746cb09797425f98578b8faef8b4a 
>   sentry-tests/pom.xml 3294335e95fb7dbb2da4151041269392004426fb 
>   sentry-tests/sentry-tests-kafka/pom.xml PRE-CREATION 
>   
> sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/CustomPrincipalBuilder.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/EmbeddedZkServer.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/KafkaTestServer.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/TestUtils.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/StaticUserGroupRole.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAclsCrud.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java
>  PRE-CREATION 
>   sentry-tests/sentry-tests-kafka/src/test/resources/log4j.properties 
> PRE-CREATION 
>   sentry-tests/sentry-tests-kafka/src/test/resources/test.crt PRE-CREATION 
>   sentry-tests/sentry-tests-kafka/src/test/resources/test.keystore.jks 
> PRE-CREATION 
>   sentry-tests/sentry-tests-kafka/src/test/resources/test.truststore.jks 
> PRE-CREATION 
>   sentry-tests/sentry-tests-kafka/src/test/resources/user1.crt PRE-CREATION 
>   sentry-tests/sentry-tests-kafka/src/test/resources/user1.keystore.jks 
> PRE-CREATION 
>   sentry-tests/sentry-tests-kafka/src/test/resources/user1.truststore.jks 
> PRE-CREATION 
>   sentry-tests/sentry-tests-kafka/src/test/resources/user2.crt PRE-CREATION 
>   sentry-tests/sentry-tests-kafka/src/test/resources/user2.keystore.jks 
> PRE-CREATION 
>   sentry-tests/sentry-tests-kafka/src/test/resources/user2.truststore.jks 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43977/diff/
> 
> 
> Testing
> -------
> 
> Added E2E tests.
> 
> Note that this contains changes from SENTRY-1098, SENTRY-1056, SENTRY-1030 
> and SENTRY-1057, and will have to be rebased once they get it.
> 
> 
> Thanks,
> 
> Ashish Singh
> 
>

Reply via email to