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 2014/05/08 00:24:57 UTC
Review Request 21178: SENTRY-175: sentry script throws error for the dbstore
service invocation
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21178/
-----------------------------------------------------------
Review request for sentry and Prasad Mujumdar.
Bugs: Sentry-175
https://issues.apache.org/jira/browse/Sentry-175
Repository: sentry
Description
-------
* Trivial fix in the script bin/sentry, arguments were not being passed correctly
* Do not fail while parsing non option arguments in SentryMain
* allow help (-h) to be passed to sub commands (service/config-tool)
Diffs
-----
bin/sentry 81b438283f1efbbb83c5699b5cfd6cbe18946677
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java bc739adf87ba7d20ada4f4273e57f31b2406d918
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/SentryMain.java eb3482b5eb4d88d7ea8bf36b3802fa2ebeebbdf7
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java bebaf0d234528e90c91d2882dd2a5767d1a61e1d
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 62584713a3f8d88d4881709357692418c882e43e
Diff: https://reviews.apache.org/r/21178/diff/
Testing
-------
bin/sentry -h
bin/sentry --command service -h
bin/sentry --command config-tool -h
bin/sentry --command service --conffile /tmp/sentry-site.xml
Thanks,
Sravya Tirukkovalur
Re: Review Request 21178: SENTRY-175: sentry script throws error for the
dbstore service invocation
Posted by Prasad Mujumdar <pr...@cloudera.com>.
> On May 7, 2014, 11:30 p.m., Prasad Mujumdar wrote:
> > bin/sentry, line 64
> > <https://reviews.apache.org/r/21178/diff/1/?file=576373#file576373line64>
> >
> > I think it's better to leave the quotes in there. It might be useful in future for options that include some special chars that otherwise be interpreted differently by the shell.
>
> Sravya Tirukkovalur wrote:
> It looks like quotes is causing a problem, making all the command line arguments into one entity. This script demonstrates it:
>
> var="1 2 3"
> for i in "$var"
> do
> echo $i
> done
> for i in $var
> do
> echo $i
> done
>
> Output:
> 1 2 3
> 1
> 2
> 3
ah ok. Thanks for verifying that.
- Prasad
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21178/#review42439
-----------------------------------------------------------
On May 7, 2014, 10:24 p.m., Sravya Tirukkovalur wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21178/
> -----------------------------------------------------------
>
> (Updated May 7, 2014, 10:24 p.m.)
>
>
> Review request for sentry and Prasad Mujumdar.
>
>
> Bugs: Sentry-175
> https://issues.apache.org/jira/browse/Sentry-175
>
>
> Repository: sentry
>
>
> Description
> -------
>
> * Trivial fix in the script bin/sentry, arguments were not being passed correctly
> * Do not fail while parsing non option arguments in SentryMain
> * allow help (-h) to be passed to sub commands (service/config-tool)
>
>
> Diffs
> -----
>
> bin/sentry 81b438283f1efbbb83c5699b5cfd6cbe18946677
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java bc739adf87ba7d20ada4f4273e57f31b2406d918
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/SentryMain.java eb3482b5eb4d88d7ea8bf36b3802fa2ebeebbdf7
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java bebaf0d234528e90c91d2882dd2a5767d1a61e1d
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 62584713a3f8d88d4881709357692418c882e43e
>
> Diff: https://reviews.apache.org/r/21178/diff/
>
>
> Testing
> -------
>
> bin/sentry -h
> bin/sentry --command service -h
> bin/sentry --command config-tool -h
> bin/sentry --command service --conffile /tmp/sentry-site.xml
>
>
> Thanks,
>
> Sravya Tirukkovalur
>
>
Re: Review Request 21178: SENTRY-175: sentry script throws error for the
dbstore service invocation
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
> On May 7, 2014, 11:30 p.m., Prasad Mujumdar wrote:
> > bin/sentry, line 64
> > <https://reviews.apache.org/r/21178/diff/1/?file=576373#file576373line64>
> >
> > I think it's better to leave the quotes in there. It might be useful in future for options that include some special chars that otherwise be interpreted differently by the shell.
It looks like quotes is causing a problem, making all the command line arguments into one entity. This script demonstrates it:
var="1 2 3"
for i in "$var"
do
echo $i
done
for i in $var
do
echo $i
done
Output:
1 2 3
1
2
3
- Sravya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21178/#review42439
-----------------------------------------------------------
On May 7, 2014, 10:24 p.m., Sravya Tirukkovalur wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21178/
> -----------------------------------------------------------
>
> (Updated May 7, 2014, 10:24 p.m.)
>
>
> Review request for sentry and Prasad Mujumdar.
>
>
> Bugs: Sentry-175
> https://issues.apache.org/jira/browse/Sentry-175
>
>
> Repository: sentry
>
>
> Description
> -------
>
> * Trivial fix in the script bin/sentry, arguments were not being passed correctly
> * Do not fail while parsing non option arguments in SentryMain
> * allow help (-h) to be passed to sub commands (service/config-tool)
>
>
> Diffs
> -----
>
> bin/sentry 81b438283f1efbbb83c5699b5cfd6cbe18946677
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java bc739adf87ba7d20ada4f4273e57f31b2406d918
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/SentryMain.java eb3482b5eb4d88d7ea8bf36b3802fa2ebeebbdf7
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java bebaf0d234528e90c91d2882dd2a5767d1a61e1d
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 62584713a3f8d88d4881709357692418c882e43e
>
> Diff: https://reviews.apache.org/r/21178/diff/
>
>
> Testing
> -------
>
> bin/sentry -h
> bin/sentry --command service -h
> bin/sentry --command config-tool -h
> bin/sentry --command service --conffile /tmp/sentry-site.xml
>
>
> Thanks,
>
> Sravya Tirukkovalur
>
>
Re: Review Request 21178: SENTRY-175: sentry script throws error for the
dbstore service invocation
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21178/#review42439
-----------------------------------------------------------
Ship it!
The patch look fine overall. Just one suggestion below.
bin/sentry
<https://reviews.apache.org/r/21178/#comment76243>
I think it's better to leave the quotes in there. It might be useful in future for options that include some special chars that otherwise be interpreted differently by the shell.
- Prasad Mujumdar
On May 7, 2014, 10:24 p.m., Sravya Tirukkovalur wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21178/
> -----------------------------------------------------------
>
> (Updated May 7, 2014, 10:24 p.m.)
>
>
> Review request for sentry and Prasad Mujumdar.
>
>
> Bugs: Sentry-175
> https://issues.apache.org/jira/browse/Sentry-175
>
>
> Repository: sentry
>
>
> Description
> -------
>
> * Trivial fix in the script bin/sentry, arguments were not being passed correctly
> * Do not fail while parsing non option arguments in SentryMain
> * allow help (-h) to be passed to sub commands (service/config-tool)
>
>
> Diffs
> -----
>
> bin/sentry 81b438283f1efbbb83c5699b5cfd6cbe18946677
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java bc739adf87ba7d20ada4f4273e57f31b2406d918
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/SentryMain.java eb3482b5eb4d88d7ea8bf36b3802fa2ebeebbdf7
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java bebaf0d234528e90c91d2882dd2a5767d1a61e1d
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 62584713a3f8d88d4881709357692418c882e43e
>
> Diff: https://reviews.apache.org/r/21178/diff/
>
>
> Testing
> -------
>
> bin/sentry -h
> bin/sentry --command service -h
> bin/sentry --command config-tool -h
> bin/sentry --command service --conffile /tmp/sentry-site.xml
>
>
> Thanks,
>
> Sravya Tirukkovalur
>
>