You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Sravya Tirukkovalur <sr...@cloudera.com> on 2013/09/20 22:37:42 UTC
Review Request 14253: Write the policy file to hdfs when using
UnmanagedHiveServer
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14253/
-----------------------------------------------------------
Review request for sentry and Shreepadma Venugopalan.
Repository: sentry
Description
-------
When using UnmanagedHiveServer, we use hdfs on the cluster and hence policy files need to be moved to hdfs
Diffs
-----
sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFile.java fafe79b
Diff: https://reviews.apache.org/r/14253/diff/
Testing
-------
All tests pass.
Thanks,
Sravya Tirukkovalur
Re: Review Request 14253: Write the policy file to hdfs when using
UnmanagedHiveServer
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
> On Sept. 21, 2013, 12:30 a.m., Shreepadma Venugopalan wrote:
> > sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFile.java, line 130
> > <https://reviews.apache.org/r/14253/diff/2/?file=355075#file355075line130>
> >
> > Path shouldn't be hard coded. Path should be an input from the user. Also what if the path doesn't exist?
Made this an input from user.
> On Sept. 21, 2013, 12:30 a.m., Shreepadma Venugopalan wrote:
> > sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFile.java, line 132
> > <https://reviews.apache.org/r/14253/diff/2/?file=355075#file355075line132>
> >
> > We should make this configurable. We may want to start HS2 with a different super user say "hiveserver2"
Made it configurable.
> On Sept. 21, 2013, 12:30 a.m., Shreepadma Venugopalan wrote:
> > sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFile.java, line 140
> > <https://reviews.apache.org/r/14253/diff/2/?file=355075#file355075line140>
> >
> > If kinit fails, we should raise an exception. Just raising a warning is insufficient because the setup has not happened correctly. We should also make keeping the policy file on HDFS configurable. We may want to run tests against an external HS2 and have the policy file on a localFS for a number of reasons.
> > Again, we shouldn't be hard coding the keytab.
Made it configurable and added another variable "sentry.e2etest.policyOnHDFS". Raising an exception if kinit fails.
> On Sept. 21, 2013, 12:30 a.m., Shreepadma Venugopalan wrote:
> > sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFile.java, line 147
> > <https://reviews.apache.org/r/14253/diff/2/?file=355075#file355075line147>
> >
> > What if this fails? We should raise an error.
> >
Remove might fail, if there is no file found at that location. Exception is thrown if the new policy file cannot be put on HDFS.
- Sravya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14253/#review26312
-----------------------------------------------------------
On Sept. 27, 2013, 8:11 p.m., Sravya Tirukkovalur wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14253/
> -----------------------------------------------------------
>
> (Updated Sept. 27, 2013, 8:11 p.m.)
>
>
> Review request for sentry and Shreepadma Venugopalan.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> When using UnmanagedHiveServer, we use hdfs on the cluster and hence policy files need to be moved to hdfs
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFile.java fafe79b
>
> Diff: https://reviews.apache.org/r/14253/diff/
>
>
> Testing
> -------
>
> All tests pass.
>
>
> Thanks,
>
> Sravya Tirukkovalur
>
>
Re: Review Request 14253: Write the policy file to hdfs when using
UnmanagedHiveServer
Posted by Shreepadma Venugopalan <sh...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14253/#review26312
-----------------------------------------------------------
sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFile.java
<https://reviews.apache.org/r/14253/#comment51405>
Path shouldn't be hard coded. Path should be an input from the user. Also what if the path doesn't exist?
sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFile.java
<https://reviews.apache.org/r/14253/#comment51403>
We should make this configurable. We may want to start HS2 with a different super user say "hiveserver2"
sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFile.java
<https://reviews.apache.org/r/14253/#comment51404>
If kinit fails, we should raise an exception. Just raising a warning is insufficient because the setup has not happened correctly. We should also make keeping the policy file on HDFS configurable. We may want to run tests against an external HS2 and have the policy file on a localFS for a number of reasons.
Again, we shouldn't be hard coding the keytab.
sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFile.java
<https://reviews.apache.org/r/14253/#comment51406>
What if this fails? We should raise an error.
- Shreepadma Venugopalan
On Sept. 21, 2013, 12:13 a.m., Sravya Tirukkovalur wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14253/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2013, 12:13 a.m.)
>
>
> Review request for sentry and Shreepadma Venugopalan.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> When using UnmanagedHiveServer, we use hdfs on the cluster and hence policy files need to be moved to hdfs
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFile.java fafe79b
>
> Diff: https://reviews.apache.org/r/14253/diff/
>
>
> Testing
> -------
>
> All tests pass.
>
>
> Thanks,
>
> Sravya Tirukkovalur
>
>
Re: Review Request 14253: Write the policy file to hdfs when using
UnmanagedHiveServer
Posted by Shreepadma Venugopalan <sh...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14253/#review26514
-----------------------------------------------------------
Ship it!
LGTM! Some minor nits.
sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFile.java
<https://reviews.apache.org/r/14253/#comment51732>
nit: java style braces.
sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFile.java
<https://reviews.apache.org/r/14253/#comment51733>
nit:java style braces
- Shreepadma Venugopalan
On Sept. 27, 2013, 8:11 p.m., Sravya Tirukkovalur wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14253/
> -----------------------------------------------------------
>
> (Updated Sept. 27, 2013, 8:11 p.m.)
>
>
> Review request for sentry and Shreepadma Venugopalan.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> When using UnmanagedHiveServer, we use hdfs on the cluster and hence policy files need to be moved to hdfs
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFile.java fafe79b
>
> Diff: https://reviews.apache.org/r/14253/diff/
>
>
> Testing
> -------
>
> All tests pass.
>
>
> Thanks,
>
> Sravya Tirukkovalur
>
>
Re: Review Request 14253: Write the policy file to hdfs when using
UnmanagedHiveServer
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14253/
-----------------------------------------------------------
(Updated Oct. 1, 2013, 2 a.m.)
Review request for sentry and Shreepadma Venugopalan.
Changes
-------
Updated the patch
Repository: sentry
Description
-------
When using UnmanagedHiveServer, we use hdfs on the cluster and hence policy files need to be moved to hdfs
Diffs (updated)
-----
sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFile.java fafe79b
Diff: https://reviews.apache.org/r/14253/diff/
Testing
-------
All tests pass.
Thanks,
Sravya Tirukkovalur
Re: Review Request 14253: Write the policy file to hdfs when using
UnmanagedHiveServer
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14253/
-----------------------------------------------------------
(Updated Sept. 27, 2013, 8:11 p.m.)
Review request for sentry and Shreepadma Venugopalan.
Changes
-------
Updated the patch with Shreepadma's suggestions. This patch is really relevant for UnmanagedHiveServer, hence I expect we may have to revisit this after all patches relating to running tests on real cluster are committed and UnmanagedHiveServer can be used.
Repository: sentry
Description
-------
When using UnmanagedHiveServer, we use hdfs on the cluster and hence policy files need to be moved to hdfs
Diffs (updated)
-----
sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFile.java fafe79b
Diff: https://reviews.apache.org/r/14253/diff/
Testing
-------
All tests pass.
Thanks,
Sravya Tirukkovalur
Re: Review Request 14253: Write the policy file to hdfs when using
UnmanagedHiveServer
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14253/
-----------------------------------------------------------
(Updated Sept. 21, 2013, 12:13 a.m.)
Review request for sentry and Shreepadma Venugopalan.
Changes
-------
Removing trailing white spaces
Repository: sentry
Description
-------
When using UnmanagedHiveServer, we use hdfs on the cluster and hence policy files need to be moved to hdfs
Diffs (updated)
-----
sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFile.java fafe79b
Diff: https://reviews.apache.org/r/14253/diff/
Testing
-------
All tests pass.
Thanks,
Sravya Tirukkovalur