> On Feb. 27, 2017, 12:52 a.m., Alexander Kolbasov wrote: > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java, > > line 26 > > <https://reviews.apache.org/r/56954/diff/1/?file=1643765#file1643765line26> > > > > This comment doesn't seem right - I do not see such two kinds of > > members.
We need to expose the configration names for priciple and rpc config name for logging puposes. Methods getSentryPrincipalConfigName and getSentryServerRpcConfigName are added for that. > On Feb. 27, 2017, 12:52 a.m., Alexander Kolbasov wrote: > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java, > > line 40 > > <https://reviews.apache.org/r/56954/diff/1/?file=1643765#file1643765line40> > > > > What are these Name() things and why do you need them? We need to expose the configration names for priciple and rpc config name for logging puposes. Methods getSentryPrincipalConfigName and getSentryServerRpcConfigName are added for that. > On Feb. 27, 2017, 12:52 a.m., Alexander Kolbasov wrote: > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java, > > line 42 > > <https://reviews.apache.org/r/56954/diff/1/?file=1643767#file1643767line42> > > > > WHy do you need this? Is it singleton? Why? I need to instantiate these classes to get polymorphic behaviour. This single instance should server the purpose of multiple sentry clients. I made it singleton so that it is not instantiated multiple times. > On Feb. 27, 2017, 12:52 a.m., Alexander Kolbasov wrote: > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java, > > line 30 > > <https://reviews.apache.org/r/56954/diff/1/?file=1643768#file1643768line30> > > > > See comments from the similar HDFS class. I need to instantiate these classes to get polymorphic behaviour. This single instance should server the purpose of multiple sentry clients. I made it singleton so that it is not instantiated multiple times. - kalyan kumar ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56954/#review166838 ----------------------------------------------------------- On Feb. 27, 2017, 11:53 p.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56954/ > ----------------------------------------------------------- > > (Updated Feb. 27, 2017, 11:53 p.m.) > > > Review request for sentry, Alexander Kolbasov, Hao Hao, Mat Crocker, Vamsee > Yarlagadda, and Vadim Spector. > > > Bugs: SENTRY-1639 > https://issues.apache.org/jira/browse/SENTRY-1639 > > > Repository: sentry > > > Description > ------- > > SENTRY-1639 Added code changes to refactor the usage of configuration > constants related to transport These are subset of changes done for > SENTRY-1592. > > > Diffs > ----- > > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java > PRE-CREATION > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java > PRE-CREATION > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java > PRE-CREATION > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java > PRE-CREATION > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/SentryConstants.java > 4ed1361 > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java > 03bf39e > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java > ee6cdf7 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java > 2dc8af8 > > Diff: https://reviews.apache.org/r/56954/diff/ > > > Testing > ------- > > > Thanks, > > kalyan kumar kalvagadda > >
