> On April 15, 2017, 12:36 a.m., Alexander Kolbasov wrote: > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransport.java > > Lines 67 (patched) > > <https://reviews.apache.org/r/57901/diff/4/?file=1692051#file1692051line67> > > > > Why do you need that? You can just the config object to constructor > > instead > > kalyan kumar kalvagadda wrote: > I want to abstract the config implementation to sentry-core package. I > don't see a need for thc client's be aware of it. All they have to say is the > type of the client it is and is handled internally.
The reason you provided the nice interface for the configuration is that no one needs to know the type of the client, they need to have the implementation of the correct configuration passed to them. You abstract this by passing an interface, Going back to creating a specific configuration there is breaking the abstraction. Why does the transport generator need to know specific type of client it is dealing with? It doesn't care! - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57901/#review172044 ----------------------------------------------------------- On April 13, 2017, 7:34 p.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57901/ > ----------------------------------------------------------- > > (Updated April 13, 2017, 7:34 p.m.) > > > Review request for sentry, Alexander Kolbasov, Hao Hao, Na Li, Vamsee > Yarlagadda, and Vadim Spector. > > > Bugs: SENTRY-1593 > https://issues.apache.org/jira/browse/SENTRY-1593 > > > Repository: sentry > > > Description > ------- > > At a high level the change set submitted does following. > 1. Extends the capability of sentry name-node client and generic policy > client to connect to multiple servers and failover to the server that is > available > 2. Transport handling of the clients is abstracted to another class so that > the client's just have to extend the class that abstracts it. Implementations > of the clients don't have to worry about it. > 3. Changed the behavior of the clients so that they connect to one of the > configured servers when the client is actually instantiated. > > As part of the code changes there are couple of files that are moved from one > package to another. Here is the list of them > 1. RetryClientInvocationHandler.java > 2. SentryClientInvocationHandler.java > 3. ThriftUtil.java > 4. SentryHdfsServiceException.java > Amount these files, RetryClientInvocationHandler.java has changed. Please > have a closer look at this file. > > > Diffs > ----- > > sentry-core/sentry-core-common/pom.xml > 9d180635ca5ed6fb1def4ffdc4addda5a650593e > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/MissingConfigurationException.java > PRE-CREATION > > 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/transport/SentryServiceClient.java > PRE-CREATION > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransport.java > PRE-CREATION > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/SentryConstants.java > 4ed1361fb1d79620c3098355a0ea9c27203a75de > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java > 23552c2512902a8500bfacb1c745ca4b10498cc8 > sentry-hdfs/sentry-hdfs-dist/pom.xml > beda2029616538101dc368b3d272fdfc942868ad > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java > 085971b34e3901b7a1d59bd8e7516b25f81ca872 > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java > ab12bf402a9a04b28e2c6c06d4e55a3607132ead > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java > 03bf39e1bed10dcd706ef12c078305b6497874f4 > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java > 2a18b1524546840df9e3275bd8a8220cc9de1b1f > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessorFactory.java > 4dc99a2bb6ec748a42ae2b65e937f6dec492fc48 > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsServiceException.java > 307d8c3172533fa768145d23664ab536dcadd6b3 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java > 919b81bd657e00841f4c8bb63a8851d1a44f8b78 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessorWrapper.java > d320d0fdb827bfcf33814a3ffa56c1898edd2521 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java > ee6cdf79f0cc96a5a72de8d79ed9c175e9aae9a3 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java > 980d9306d848d54fd476252793afc4c3a7fc6a08 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java > f6bb8a522217bc26d768026f1c6be8c3e758fad8 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java > fb7c40a4d32f6c5588b277c637e041ca8b3d999e > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java > 8cf1c1a16a26939f4cf68278c7b8204ec0b08ca7 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java > c2b03e5d62b88bd5c2f696af8f8872c42c02de9c > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java > 2cf748e1e538c99fc34c57ed50b78701977c84ad > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java > ee2a4662c1c564dda8d050340e6b06cdce516530 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryProcessorWrapper.java > a5f11a98f1bdb5bc9bd6c2c86b6fa40413b94bca > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/ThriftUtil.java > 5fed04ad079869c77d7b60085db6ba7791f586ab > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java > d5f4fcb56d05dc699634927106f6fa31e74fa213 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/RetryClientInvocationHandler.java > c4964c34f6714c96d578bed49efc68d7f13e5925 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryClientInvocationHandler.java > b8c7f23232f887b9c00662f7cef01e1a72c56eb7 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java > f822497d9fdc8536d4bb9f52fa3d0f69acf78908 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java > d3a68c9696d12cf353b59aa9b877f7ee971caf25 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java > d4bf4359c0d057f441ddd1f738f5468eaee5e385 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceClient.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java > 1ec884041b361df90e7186f05d0c964e1ba89fc4 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactoryGM.java > dfae5abaf6f3e52ca7d0a3a7862b0d39b6445016 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClient.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java > 04d92ddc56932e49e555c88136255ce234738dbe > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java > dfd79aeb998d32f1f50e14328977ea1946d13029 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java > 729238707b9b78776e9db996de104c8f13f843bf > > > Diff: https://reviews.apache.org/r/57901/diff/4/ > > > Testing > ------- > > > Thanks, > > kalyan kumar kalvagadda > >
