> On Feb. 6, 2016, 4:51 a.m., Hao Hao wrote:
> > sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/authorizer/SentryKafkaAuthorizer.java,
> >  line 72
> > <https://reviews.apache.org/r/43064/diff/4/?file=1235952#file1235952line72>
> >
> >     Will the service name always be kafka? Thought it will be different 
> > based on which service instance it is actually coming from.

That is a valid concern. However, at this point I have no way of 
differentiating incoming requests based on source Kafka cluster. Only info we 
have is session, operation and resource for incoming authorize requests. In 
future, we might address this in future if the need be. However, for now we can 
just go ahead with assumption that we have just one Kafka cluster per Sentry 
instance.


> On Feb. 6, 2016, 4:51 a.m., Hao Hao wrote:
> > sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/authorizer/SentryKafkaAuthorizer.java,
> >  line 159
> > <https://reviews.apache.org/r/43064/diff/4/?file=1235952#file1235952line159>
> >
> >     Rename to getAclsForAllRoles?

This is an overriden method, can't rename.


> On Feb. 6, 2016, 4:51 a.m., Hao Hao wrote:
> > sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/authorizer/SentryKafkaAuthorizer.java,
> >  line 178
> > <https://reviews.apache.org/r/43064/diff/4/?file=1235952#file1235952line178>
> >
> >     Rename to getAclsForAllRoles?

This is an overriden method, can't rename.


> On Feb. 6, 2016, 4:51 a.m., Hao Hao wrote:
> > sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java,
> >  line 57
> > <https://reviews.apache.org/r/43064/diff/4/?file=1235953#file1235953line57>
> >
> >     Remove?

Removed.


- Ashish


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


On Feb. 5, 2016, 5:01 a.m., Ashish Singh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43064/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2016, 5:01 a.m.)
> 
> 
> Review request for sentry and Dapeng Sun.
> 
> 
> Bugs: SENTRY-1011
>     https://issues.apache.org/jira/browse/SENTRY-1011
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1011: Add Kafka binding
> 
> 
> Diffs
> -----
> 
>   pom.xml aa99e313d8ae98bf048ce7d78574a9398f22732e 
>   sentry-binding/pom.xml 0f2a987668cb3ea13d921636dfbc063bd17a3790 
>   sentry-binding/sentry-binding-kafka/pom.xml PRE-CREATION 
>   
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/ConvertUtil.java
>  PRE-CREATION 
>   
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/authorizer/SentryKafkaAuthorizer.java
>  PRE-CREATION 
>   
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java
>  PRE-CREATION 
>   
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBindingSingleton.java
>  PRE-CREATION 
>   
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/conf/KafkaAuthConf.java
>  PRE-CREATION 
>   
> sentry-binding/sentry-binding-kafka/src/test/java/org/apache/sentry/kafka/MockGroupMappingServiceProvider.java
>  PRE-CREATION 
>   
> sentry-binding/sentry-binding-kafka/src/test/java/org/apache/sentry/kafka/authorizer/ConvertUtilTest.java
>  PRE-CREATION 
>   
> sentry-binding/sentry-binding-kafka/src/test/java/org/apache/sentry/kafka/authorizer/SentryKafkaAuthorizerTest.java
>  PRE-CREATION 
>   sentry-binding/sentry-binding-kafka/src/test/resources/core-site.xml 
> PRE-CREATION 
>   sentry-binding/sentry-binding-kafka/src/test/resources/log4j.properties 
> PRE-CREATION 
>   sentry-binding/sentry-binding-kafka/src/test/resources/sentry-site.xml 
> PRE-CREATION 
>   
> sentry-binding/sentry-binding-kafka/src/test/resources/test-authz-provider.ini
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationComponent.java
>  6409015a92c94d37f21e0dbc8f7d7de74e3be5ec 
> 
> Diff: https://reviews.apache.org/r/43064/diff/
> 
> 
> Testing
> -------
> 
> Tested with unit and e2e tests.
> 
> 
> Thanks,
> 
> Ashish Singh
> 
>

Reply via email to