OK I can add a switch to call the validation code for the shell only. Do
you know why the CommandUtil includes:
tSentryPrivilege.setAction(AccessConstants.ALL);
for the URI case? This looks a bit dodgy to me as it would override the
value parsed via PRIVILEGE_ACTION_NAME.
Colm.
On Wed, Oct 4, 2017 at 11:37 PM, Alexander Kolbasov <ak...@cloudera.com>
wrote:
> Interesting - both were introduced by Colin Ma. I definitely agree that we
> do not need two duplicates of the same similar functionality. The exception
> part is interesting - the validator throws unchecked exceptions.
>
> The version in SentryServiceUtil is used only in import policy code path
> where validation should be always ok.
>
> The second one is used by shell only.
>
> There is another internal version in SentryStore:
>
> private void convertToTSentryPrivilege(MSentryPrivilege mSentryPrivilege,
> TSentryPrivilege privilege) {
> privilege.setCreateTime(mSentryPrivilege.getCreateTime());
> privilege.setAction(fromNULLCol(mSentryPrivilege.getAction()));
> privilege.setPrivilegeScope(mSentryPrivilege.getPrivilegeScope());
> privilege.setServerName(fromNULLCol(mSentryPrivilege.getServerName()));
> privilege.setDbName(fromNULLCol(mSentryPrivilege.getDbName()));
> privilege.setTableName(fromNULLCol(mSentryPrivilege.getTableName()));
> privilege.setColumnName(fromNULLCol(mSentryPrivilege.getColumnName()));
> privilege.setURI(fromNULLCol(mSentryPrivilege.getURI()));
> if (mSentryPrivilege.getGrantOption() != null) {
> privilege.setGrantOption(TSentryGrantOption.valueOf(mSentryPrivilege.
> getGrantOption().toString().toUpperCase()));
> } else {
> privilege.setGrantOption(TSentryGrantOption.UNSET);
> }
> }
>
>
>
> On Mon, Oct 2, 2017 at 10:07 AM, Colm O hEigeartaigh <cohei...@apache.org>
> wrote:
>
> > Hi all,
> >
> > I'm preparing a patch to consolidate some duplicate code in
> > sentry-provider-db, and have a question:
> >
> > CommandUtil.convertToTSentryPrivilege:
> >
> > https://github.com/apache/sentry/blob/186e75543a23f286e24c56f7385909
> > 9405b7b73a/sentry-provider/sentry-provider-db/src/main/
> > java/org/apache/sentry/provider/db/tools/command/
> hive/CommandUtil.java#L38
> >
> > is identical to SentryServiceUtil.convertToTSentryPrivilege:
> >
> > https://github.com/apache/sentry/blob/186e75543a23f286e24c56f7385909
> > 9405b7b73a/sentry-provider/sentry-provider-db/src/main/
> > java/org/apache/sentry/service/thrift/SentryServiceUtil.java#L56
> >
> > apart from two differences:
> >
> > a) The CommandUtil version sets:
> > tSentryPrivilege.setAction(AccessConstants.ALL); for the
> > PRIVILEGE_URI_NAME case.
> > b) The CommandUtil version "validates" the privilege hierarchy, throwing
> an
> > exception if one of the names is not specified.
> >
> > Ideally I'd like to remove CommandUtil altogether and just reference the
> > methods in SentryServiceUtil. Should these extra pieces also be in
> > SentryServiceUtil? Or is there a reason that they are different?
> >
> > Colm.
> >
> >
> > --
> > Colm O hEigeartaigh
> >
> > Talend Community Coder
> > http://coders.talend.com
> >
>
--
Colm O hEigeartaigh
Talend Community Coder
http://coders.talend.com
