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
>
