Re: Review Request 65867: SENTRY-2147 - Fix Javadoc for SentryHiveAuthorizerFactory

2018-03-05 Thread Steve Moist via Review Board 

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


Ship it!




Ship It!

- Steve Moist


On March 5, 2018, 5:25 p.m., Colm O hEigeartaigh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65867/
> ---
> 
> (Updated March 5, 2018, 5:25 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: SENTRY-2147
> https://issues.apache.org/jira/browse/SENTRY-2147
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> The Javadoc for SentryHiveAuthorizerFactory incorrectly states that it should 
> be configured as follows:
> 
>  
>hive.security.authorization.enable
>   
> org.apache.sentry.binding.hive.authz.SentryHiveAuthorizerFactory
> 
> 
> Instead it should be "hive.security.authorization.manager".
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryHiveAuthorizerFactory.java
>  db8e4288 
> 
> 
> Diff: https://reviews.apache.org/r/65867/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Colm O hEigeartaigh
> 
>

Re: Review Request 65867: SENTRY-2147 - Fix Javadoc for SentryHiveAuthorizerFactory

2018-03-05 Thread kalyan kumar kalvagadda via Review Board 

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


Ship it!




Ship It!

- kalyan kumar kalvagadda


On March 5, 2018, 5:25 p.m., Colm O hEigeartaigh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65867/
> ---
> 
> (Updated March 5, 2018, 5:25 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: SENTRY-2147
> https://issues.apache.org/jira/browse/SENTRY-2147
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> The Javadoc for SentryHiveAuthorizerFactory incorrectly states that it should 
> be configured as follows:
> 
>  
>hive.security.authorization.enable
>   
> org.apache.sentry.binding.hive.authz.SentryHiveAuthorizerFactory
> 
> 
> Instead it should be "hive.security.authorization.manager".
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryHiveAuthorizerFactory.java
>  db8e4288 
> 
> 
> Diff: https://reviews.apache.org/r/65867/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Colm O hEigeartaigh
> 
>

Re: Review Request 65867: SENTRY-2147 - Fix Javadoc for SentryHiveAuthorizerFactory

2018-03-05 Thread Colm O hEigeartaigh 

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

(Updated March 5, 2018, 5:25 p.m.)


Review request for sentry.


Bugs: SENTRY-2147
https://issues.apache.org/jira/browse/SENTRY-2147


Repository: sentry


Description
---

The Javadoc for SentryHiveAuthorizerFactory incorrectly states that it should 
be configured as follows:

 
   hive.security.authorization.enable
  
org.apache.sentry.binding.hive.authz.SentryHiveAuthorizerFactory


Instead it should be "hive.security.authorization.manager".


Diffs (updated)
-

  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryHiveAuthorizerFactory.java
 db8e4288 


Diff: https://reviews.apache.org/r/65867/diff/2/

Changes: https://reviews.apache.org/r/65867/diff/1-2/


Testing
---


Thanks,

Colm O hEigeartaigh

Re: Review Request 65867: SENTRY-2147 - Fix Javadoc for SentryHiveAuthorizerFactory

2018-03-05 Thread Colm O hEigeartaigh 


> On March 2, 2018, 3:43 p.m., Na Li wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryHiveAuthorizerFactory.java
> > Line 34 (original), 34 (patched)
> > 
> >
> > can you use "the hive-site.xml at hive server 2" instead of 
> > "hive-site.xml"? hive-site.xml can be at both hive server 2, HMS, or at 
> > sentry.
> 
> Colm O hEigeartaigh wrote:
> It's hiveserver2-site.xml though not hive-site.xml?
> 
> Na Li wrote:
> In our unit tests and e2e tests, the hive configuration files are always 
> called hive-site.xml. Would it be confusing for someone to see 
> hiveserver2-site.xml? I searched the code, there is no other places refer 
> such file name.

OK it's probably simpler just to leave it as "hive-site.xml".


- Colm


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


On March 1, 2018, 12:32 p.m., Colm O hEigeartaigh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65867/
> ---
> 
> (Updated March 1, 2018, 12:32 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: SENTRY-2147
> https://issues.apache.org/jira/browse/SENTRY-2147
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> The Javadoc for SentryHiveAuthorizerFactory incorrectly states that it should 
> be configured as follows:
> 
>  
>hive.security.authorization.enable
>   
> org.apache.sentry.binding.hive.authz.SentryHiveAuthorizerFactory
> 
> 
> Instead it should be "hive.security.authorization.manager".
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryHiveAuthorizerFactory.java
>  db8e4288 
> 
> 
> Diff: https://reviews.apache.org/r/65867/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Colm O hEigeartaigh
> 
>

Re: Review Request 65867: SENTRY-2147 - Fix Javadoc for SentryHiveAuthorizerFactory

2018-03-05 Thread Colm O hEigeartaigh 


> On March 2, 2018, 7:34 p.m., kalyan kumar kalvagadda wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryHiveAuthorizerFactory.java
> > Line 34 (original), 34 (patched)
> > 
> >
> > Colm,
> > 
> > Are you sre that this configuration is not needed for metastore?

Yeah I've removed the change to the wording on "hive-site.xml". Now the patch 
just has the correct tag to use.


- Colm


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


On March 1, 2018, 12:32 p.m., Colm O hEigeartaigh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65867/
> ---
> 
> (Updated March 1, 2018, 12:32 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: SENTRY-2147
> https://issues.apache.org/jira/browse/SENTRY-2147
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> The Javadoc for SentryHiveAuthorizerFactory incorrectly states that it should 
> be configured as follows:
> 
>  
>hive.security.authorization.enable
>   
> org.apache.sentry.binding.hive.authz.SentryHiveAuthorizerFactory
> 
> 
> Instead it should be "hive.security.authorization.manager".
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryHiveAuthorizerFactory.java
>  db8e4288 
> 
> 
> Diff: https://reviews.apache.org/r/65867/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Colm O hEigeartaigh
> 
>

Re: Review Request 65867: SENTRY-2147 - Fix Javadoc for SentryHiveAuthorizerFactory

2018-03-02 Thread Na Li via Review Board 


> On March 2, 2018, 3:43 p.m., Na Li wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryHiveAuthorizerFactory.java
> > Line 34 (original), 34 (patched)
> > 
> >
> > can you use "the hive-site.xml at hive server 2" instead of 
> > "hive-site.xml"? hive-site.xml can be at both hive server 2, HMS, or at 
> > sentry.
> 
> Colm O hEigeartaigh wrote:
> It's hiveserver2-site.xml though not hive-site.xml?

In our unit tests and e2e tests, the hive configuration files are always called 
hive-site.xml. Would it be confusing for someone to see hiveserver2-site.xml? I 
searched the code, there is no other places refer such file name.


- Na


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


On March 1, 2018, 12:32 p.m., Colm O hEigeartaigh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65867/
> ---
> 
> (Updated March 1, 2018, 12:32 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: SENTRY-2147
> https://issues.apache.org/jira/browse/SENTRY-2147
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> The Javadoc for SentryHiveAuthorizerFactory incorrectly states that it should 
> be configured as follows:
> 
>  
>hive.security.authorization.enable
>   
> org.apache.sentry.binding.hive.authz.SentryHiveAuthorizerFactory
> 
> 
> Instead it should be "hive.security.authorization.manager".
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryHiveAuthorizerFactory.java
>  db8e4288 
> 
> 
> Diff: https://reviews.apache.org/r/65867/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Colm O hEigeartaigh
> 
>

Re: Review Request 65867: SENTRY-2147 - Fix Javadoc for SentryHiveAuthorizerFactory

2018-03-02 Thread kalyan kumar kalvagadda via Review Board 

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




sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryHiveAuthorizerFactory.java
Line 34 (original), 34 (patched)


Colm,

Are you sre that this configuration is not needed for metastore?


- kalyan kumar kalvagadda


On March 1, 2018, 12:32 p.m., Colm O hEigeartaigh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65867/
> ---
> 
> (Updated March 1, 2018, 12:32 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: SENTRY-2147
> https://issues.apache.org/jira/browse/SENTRY-2147
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> The Javadoc for SentryHiveAuthorizerFactory incorrectly states that it should 
> be configured as follows:
> 
>  
>hive.security.authorization.enable
>   
> org.apache.sentry.binding.hive.authz.SentryHiveAuthorizerFactory
> 
> 
> Instead it should be "hive.security.authorization.manager".
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryHiveAuthorizerFactory.java
>  db8e4288 
> 
> 
> Diff: https://reviews.apache.org/r/65867/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Colm O hEigeartaigh
> 
>

Re: Review Request 65867: SENTRY-2147 - Fix Javadoc for SentryHiveAuthorizerFactory

2018-03-02 Thread Colm O hEigeartaigh 


> On March 2, 2018, 3:43 p.m., Na Li wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryHiveAuthorizerFactory.java
> > Line 34 (original), 34 (patched)
> > 
> >
> > can you use "the hive-site.xml at hive server 2" instead of 
> > "hive-site.xml"? hive-site.xml can be at both hive server 2, HMS, or at 
> > sentry.

It's hiveserver2-site.xml though not hive-site.xml?


- Colm


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


On March 1, 2018, 12:32 p.m., Colm O hEigeartaigh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65867/
> ---
> 
> (Updated March 1, 2018, 12:32 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: SENTRY-2147
> https://issues.apache.org/jira/browse/SENTRY-2147
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> The Javadoc for SentryHiveAuthorizerFactory incorrectly states that it should 
> be configured as follows:
> 
>  
>hive.security.authorization.enable
>   
> org.apache.sentry.binding.hive.authz.SentryHiveAuthorizerFactory
> 
> 
> Instead it should be "hive.security.authorization.manager".
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryHiveAuthorizerFactory.java
>  db8e4288 
> 
> 
> Diff: https://reviews.apache.org/r/65867/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Colm O hEigeartaigh
> 
>

Re: Review Request 65867: SENTRY-2147 - Fix Javadoc for SentryHiveAuthorizerFactory

2018-03-02 Thread Na Li via Review Board 

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




sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryHiveAuthorizerFactory.java
Line 34 (original), 34 (patched)


can you use "the hive-site.xml at hive server 2" instead of 
"hive-site.xml"? hive-site.xml can be at both hive server 2, HMS, or at sentry.


- Na Li


On March 1, 2018, 12:32 p.m., Colm O hEigeartaigh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65867/
> ---
> 
> (Updated March 1, 2018, 12:32 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: SENTRY-2147
> https://issues.apache.org/jira/browse/SENTRY-2147
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> The Javadoc for SentryHiveAuthorizerFactory incorrectly states that it should 
> be configured as follows:
> 
>  
>hive.security.authorization.enable
>   
> org.apache.sentry.binding.hive.authz.SentryHiveAuthorizerFactory
> 
> 
> Instead it should be "hive.security.authorization.manager".
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryHiveAuthorizerFactory.java
>  db8e4288 
> 
> 
> Diff: https://reviews.apache.org/r/65867/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Colm O hEigeartaigh
> 
>

Re: Review Request 65867: SENTRY-2147 - Fix Javadoc for SentryHiveAuthorizerFactory

2018-03-02 Thread Na Li via Review Board 

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


Ship it!




Ship It!

- Na Li


On March 1, 2018, 12:32 p.m., Colm O hEigeartaigh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65867/
> ---
> 
> (Updated March 1, 2018, 12:32 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: SENTRY-2147
> https://issues.apache.org/jira/browse/SENTRY-2147
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> The Javadoc for SentryHiveAuthorizerFactory incorrectly states that it should 
> be configured as follows:
> 
>  
>hive.security.authorization.enable
>   
> org.apache.sentry.binding.hive.authz.SentryHiveAuthorizerFactory
> 
> 
> Instead it should be "hive.security.authorization.manager".
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryHiveAuthorizerFactory.java
>  db8e4288 
> 
> 
> Diff: https://reviews.apache.org/r/65867/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Colm O hEigeartaigh
> 
>