You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Prasad Mujumdar <pr...@cloudera.com> on 2014/09/19 01:00:36 UTC
Review Request 25799: SENTRY-430: Sentry Service does not use correct
classpath when HIVE_HOME environment var is defined
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25799/
-----------------------------------------------------------
Review request for sentry, Lenni Kuff and Sravya Tirukkovalur.
Bugs: SENTRY-430
https://issues.apache.org/jira/browse/SENTRY-430
Repository: sentry
Description
-------
Separate out the config-tool into a separate script. This way the core sentry options don't need any dependancy on Hive.
Diffs
-----
bin/config_tool PRE-CREATION
bin/sentry 2990ef4
Diff: https://reviews.apache.org/r/25799/diff/
Testing
-------
Manually tested various Sentry script option with the patch
Thanks,
Prasad Mujumdar
Re: Review Request 25799: SENTRY-430: Sentry Service does not use
correct classpath when HIVE_HOME environment var is defined
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25799/
-----------------------------------------------------------
(Updated Sept. 19, 2014, 1:28 a.m.)
Review request for sentry, Lenni Kuff and Sravya Tirukkovalur.
Changes
-------
Restrict direct invocation of config tool script.
Bugs: SENTRY-430
https://issues.apache.org/jira/browse/SENTRY-430
Repository: sentry
Description
-------
Separate out the config-tool into a separate script. This way the core sentry options don't need any dependancy on Hive.
Diffs (updated)
-----
bin/config_tool PRE-CREATION
bin/sentry 2990ef4
Diff: https://reviews.apache.org/r/25799/diff/
Testing
-------
Manually tested various Sentry script option with the patch
Thanks,
Prasad Mujumdar
Re: Review Request 25799: SENTRY-430: Sentry Service does not use
correct classpath when HIVE_HOME environment var is defined
Posted by Prasad Mujumdar <pr...@cloudera.com>.
> On Sept. 19, 2014, 1:10 a.m., Lenni Kuff wrote:
> > bin/config_tool, line 49
> > <https://reviews.apache.org/r/25799/diff/2/?file=694227#file694227line49>
> >
> > hmmm.. Since this now relies on _CMD_JAR being exported, I don't think this script can run on its own anymore. Consider moving the classpath setup stuff back in here or adding a check to ensure _CMD_JAR has been print a relevant error message. Either one seems reasonable.
Even before this change, the config_tool was not intended to be a standalone script. It was not setting SENTRY_HOME, hadoop path etc. The idea is to keep the sentry script as the top level interface and not to change the existing behavior.
Perhaps we can export RUN_CONFIG_TOOL and verify that setting to make sure that the script is not directly invoked.
- Prasad
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25799/#review53914
-----------------------------------------------------------
On Sept. 19, 2014, 1:07 a.m., Prasad Mujumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25799/
> -----------------------------------------------------------
>
> (Updated Sept. 19, 2014, 1:07 a.m.)
>
>
> Review request for sentry, Lenni Kuff and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-430
> https://issues.apache.org/jira/browse/SENTRY-430
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Separate out the config-tool into a separate script. This way the core sentry options don't need any dependancy on Hive.
>
>
> Diffs
> -----
>
> bin/config_tool PRE-CREATION
> bin/sentry 2990ef4
>
> Diff: https://reviews.apache.org/r/25799/diff/
>
>
> Testing
> -------
>
> Manually tested various Sentry script option with the patch
>
>
> Thanks,
>
> Prasad Mujumdar
>
>
Re: Review Request 25799: SENTRY-430: Sentry Service does not use
correct classpath when HIVE_HOME environment var is defined
Posted by Lenni Kuff <ls...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25799/#review53914
-----------------------------------------------------------
bin/config_tool
<https://reviews.apache.org/r/25799/#comment93785>
hmmm.. Since this now relies on _CMD_JAR being exported, I don't think this script can run on its own anymore. Consider moving the classpath setup stuff back in here or adding a check to ensure _CMD_JAR has been print a relevant error message. Either one seems reasonable.
- Lenni Kuff
On Sept. 19, 2014, 1:07 a.m., Prasad Mujumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25799/
> -----------------------------------------------------------
>
> (Updated Sept. 19, 2014, 1:07 a.m.)
>
>
> Review request for sentry, Lenni Kuff and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-430
> https://issues.apache.org/jira/browse/SENTRY-430
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Separate out the config-tool into a separate script. This way the core sentry options don't need any dependancy on Hive.
>
>
> Diffs
> -----
>
> bin/config_tool PRE-CREATION
> bin/sentry 2990ef4
>
> Diff: https://reviews.apache.org/r/25799/diff/
>
>
> Testing
> -------
>
> Manually tested various Sentry script option with the patch
>
>
> Thanks,
>
> Prasad Mujumdar
>
>
Re: Review Request 25799: SENTRY-430: Sentry Service does not use
correct classpath when HIVE_HOME environment var is defined
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25799/
-----------------------------------------------------------
(Updated Sept. 19, 2014, 1:07 a.m.)
Review request for sentry, Lenni Kuff and Sravya Tirukkovalur.
Changes
-------
Addresses review feedback
Bugs: SENTRY-430
https://issues.apache.org/jira/browse/SENTRY-430
Repository: sentry
Description
-------
Separate out the config-tool into a separate script. This way the core sentry options don't need any dependancy on Hive.
Diffs (updated)
-----
bin/config_tool PRE-CREATION
bin/sentry 2990ef4
Diff: https://reviews.apache.org/r/25799/diff/
Testing
-------
Manually tested various Sentry script option with the patch
Thanks,
Prasad Mujumdar
Re: Review Request 25799: SENTRY-430: Sentry Service does not use
correct classpath when HIVE_HOME environment var is defined
Posted by Lenni Kuff <ls...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25799/#review53903
-----------------------------------------------------------
Ship it!
- Lenni Kuff
On Sept. 18, 2014, 11 p.m., Prasad Mujumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25799/
> -----------------------------------------------------------
>
> (Updated Sept. 18, 2014, 11 p.m.)
>
>
> Review request for sentry, Lenni Kuff and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-430
> https://issues.apache.org/jira/browse/SENTRY-430
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Separate out the config-tool into a separate script. This way the core sentry options don't need any dependancy on Hive.
>
>
> Diffs
> -----
>
> bin/config_tool PRE-CREATION
> bin/sentry 2990ef4
>
> Diff: https://reviews.apache.org/r/25799/diff/
>
>
> Testing
> -------
>
> Manually tested various Sentry script option with the patch
>
>
> Thanks,
>
> Prasad Mujumdar
>
>
Re: Review Request 25799: SENTRY-430: Sentry Service does not use
correct classpath when HIVE_HOME environment var is defined
Posted by Prasad Mujumdar <pr...@cloudera.com>.
> On Sept. 19, 2014, 12:14 a.m., Lenni Kuff wrote:
> > bin/config_tool, line 45
> > <https://reviews.apache.org/r/25799/diff/1/?file=694117#file694117line45>
> >
> > Curious - is there something special about exit code 5?
Not really. The existing Sentry script had exit code till 3, so just something higer .. :)
> On Sept. 19, 2014, 12:14 a.m., Lenni Kuff wrote:
> > bin/sentry, line 20
> > <https://reviews.apache.org/r/25799/diff/1/?file=694118#file694118line20>
> >
> > Maybe make this a binary (RUN_CONFIG_TOOL=0/1) rather than a string y/n.
sure. will change
> On Sept. 19, 2014, 12:14 a.m., Lenni Kuff wrote:
> > bin/sentry, line 70
> > <https://reviews.apache.org/r/25799/diff/1/?file=694118#file694118line70>
> >
> > this code of setting up the classpath and the CMD_JAR can get pushed inside the if block since you also do this in the config_tool script.
Removed it from config-tool and export CMD_JAR in sentry script.
- Prasad
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25799/#review53901
-----------------------------------------------------------
On Sept. 18, 2014, 11 p.m., Prasad Mujumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25799/
> -----------------------------------------------------------
>
> (Updated Sept. 18, 2014, 11 p.m.)
>
>
> Review request for sentry, Lenni Kuff and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-430
> https://issues.apache.org/jira/browse/SENTRY-430
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Separate out the config-tool into a separate script. This way the core sentry options don't need any dependancy on Hive.
>
>
> Diffs
> -----
>
> bin/config_tool PRE-CREATION
> bin/sentry 2990ef4
>
> Diff: https://reviews.apache.org/r/25799/diff/
>
>
> Testing
> -------
>
> Manually tested various Sentry script option with the patch
>
>
> Thanks,
>
> Prasad Mujumdar
>
>
Re: Review Request 25799: SENTRY-430: Sentry Service does not use
correct classpath when HIVE_HOME environment var is defined
Posted by Lenni Kuff <ls...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25799/#review53901
-----------------------------------------------------------
bin/config_tool
<https://reviews.apache.org/r/25799/#comment93767>
Curious - is there something special about exit code 5?
bin/sentry
<https://reviews.apache.org/r/25799/#comment93765>
Maybe make this a binary (RUN_CONFIG_TOOL=0/1) rather than a string y/n.
bin/sentry
<https://reviews.apache.org/r/25799/#comment93768>
this code of setting up the classpath and the CMD_JAR can get pushed inside the if block since you also do this in the config_tool script.
- Lenni Kuff
On Sept. 18, 2014, 11 p.m., Prasad Mujumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25799/
> -----------------------------------------------------------
>
> (Updated Sept. 18, 2014, 11 p.m.)
>
>
> Review request for sentry, Lenni Kuff and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-430
> https://issues.apache.org/jira/browse/SENTRY-430
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Separate out the config-tool into a separate script. This way the core sentry options don't need any dependancy on Hive.
>
>
> Diffs
> -----
>
> bin/config_tool PRE-CREATION
> bin/sentry 2990ef4
>
> Diff: https://reviews.apache.org/r/25799/diff/
>
>
> Testing
> -------
>
> Manually tested various Sentry script option with the patch
>
>
> Thanks,
>
> Prasad Mujumdar
>
>