> On Dec. 22, 2015, 10:23 p.m., Li Li wrote: > > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/hadoop/hdfs/server/namenode/AuthorizationProvider.java, > > line 177 > > <https://reviews.apache.org/r/41349/diff/1/?file=1162395#file1162395line177> > > > > seems PMD cannot catch the trailing space :-)
No, but the checkstyle plugin can - I'll take a look at that next. > On Dec. 22, 2015, 10:23 p.m., Li Li wrote: > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ha/HdfsHAClientInvocationHandler.java, > > line 62 > > <https://reviews.apache.org/r/41349/diff/1/?file=1162392#file1162392line62> > > > > In the original code, it will retry for some exceptions, while after > > removing 'while(true)' it will never retry. I guess it may trigger some > > test failure. > > Also I wonder why we do not have any limitation for the retry, eg. no > > maxRetryTimes or timeout. Maybe I'm missing something, but I don't see how the current code retries for some exceptions. The last statement in the loop is "return result;" so it will always return the result after the first iteration, regardless of whether there was an exception or not. - Colm ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41349/#review111666 ----------------------------------------------------------- On Dec. 21, 2015, 11:01 a.m., Colm O hEigeartaigh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41349/ > ----------------------------------------------------------- > > (Updated Dec. 21, 2015, 11:01 a.m.) > > > Review request for sentry. > > > Repository: sentry > > > Description > ------- > > Apply PMD plugin to Sentry source > > > Diffs > ----- > > pom.xml 9495286 > > sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/authorizer/DefaultSentryValidator.java > 2bc8aad > > sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/AuthorizingObjectStoreV2.java > ff648ff > > sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetastorePostEventListenerV2.java > a72e745 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java > d47ca3b > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java > 1e2b3b9 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryHivePrivilegeObjectDesc.java > 8929357 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java > 994af8a > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java > a51653c > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java > 5898b7e > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryOnFailureHookContext.java > a380651 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryPolicyFileFormatter.java > 14437ca > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java > 926c46c > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java > d35b09d > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java > 2e0f299 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java > e76fad1 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStore.java > 9938373 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBinding.java > f6b9c7a > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryHiveMetaStoreClient.java > 6a33ef9 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetaStoreFilterHook.java > 9f33f3d > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java > 3c8ad1f > > sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java > 6980c7c > > sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/authz/SentryAuthorizationValidator.java > 5f96767 > > sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java > 42638f8 > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/Command.java > 528f7d7 > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/SentryMain.java > e081a86 > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/Action.java > 1479e5c > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/Authorizable.java > 3523237 > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/BitFieldAction.java > 5aa0f83 > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PathUtils.java > 6cb599c > > sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/DBModelAuthorizable.java > 4d74356 > > sentry-core/sentry-core-model-indexer/src/main/java/org/apache/sentry/core/model/indexer/IndexerModelAuthorizable.java > d92a5c8 > > sentry-core/sentry-core-model-indexer/src/test/java/org/apache/sentry/core/indexer/TestIndexerBitFieldAction.java > a490cd8 > > sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/SearchModelAuthorizable.java > d6a9d54 > > sentry-core/sentry-core-model-search/src/test/java/org/apache/sentry/core/search/TestSearchBitFieldAction.java > 0ae49d6 > > sentry-core/sentry-core-model-sqoop/src/main/java/org/apache/sentry/core/model/sqoop/SqoopActionFactory.java > c1f33ec > > sentry-core/sentry-core-model-sqoop/src/main/java/org/apache/sentry/core/model/sqoop/SqoopAuthorizable.java > b57f4a7 > > sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/SentryHDFSService.java > 663fe4e > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/AuthzPaths.java > ba16f4a > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/AuthzPathsDumper.java > 2bd2a88 > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/AuthzPermissions.java > 1631ae5 > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java > d52e361 > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java > 956b855 > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java > ac8459b > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPaths.java > b74f954 > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ha/HdfsHAClientInvocationHandler.java > ec66b2d > > sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestHMSPathsFullDump.java > d01f7dd > > sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestKrbConnectionTimeout.java > 2db72b1 > > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/hadoop/hdfs/server/namenode/AuthorizationProvider.java > db3d413 > > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationConstants.java > 25fd71c > > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java > c9accc1 > > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationProvider.java > 4d03ba3 > > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryPermissions.java > 2c50ea9 > > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java > 422554e > > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPermissions.java > aa78360 > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java > 4349c6e > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java > f88295d > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java > 647e8fc > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java > 22a436a > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarderWithHA.java > 9a4e7bb > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateablePermissions.java > 2fe81fd > > sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestHAUpdateForwarder.java > 40af05a > > sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/PolicyEngine.java > 38a5b65 > > sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/Privilege.java > c7e1734 > > sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/PrivilegeValidator.java > 5548f04 > > sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/DBWildcardPrivilege.java > 939d9ec > > sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/SimpleDBPolicyEngine.java > a03794e > > sentry-policy/sentry-policy-search/src/test/java/org/apache/sentry/policy/search/TestSearchAuthorizationProviderGeneralCases.java > bdb1c96 > > sentry-policy/sentry-policy-sqoop/src/main/java/org/apache/sentry/policy/sqoop/ServerNameRequiredMatch.java > 3a57dfc > > sentry-provider/sentry-provider-cache/src/main/java/org/apache/sentry/provider/cache/PrivilegeCache.java > 29c6c5c > > sentry-provider/sentry-provider-cache/src/main/java/org/apache/sentry/provider/cache/SimpleCacheProviderBackend.java > 4b98447 > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java > fe54b42 > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/GroupMappingService.java > 22371d1 > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/HadoopGroupMappingService.java > 4214449 > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/HadoopGroupResourceAuthorizationProvider.java > c8e6c9d > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/NoAuthorizationProvider.java > 7cf617e > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ProviderBackend.java > ddb9cf9 > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java > 7bf830c > > sentry-provider/sentry-provider-common/src/test/java/org/apache/sentry/provider/common/TestGetGroupMapping.java > f57198a > > sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyService.java > b421598 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SentryPolicyStorePlugin.java > 998a48b > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java > ff25d95 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java > d7cb814 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeObject.java > aa56207 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java > 98b22b0 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java > ba9e36f > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/NotificationHandler.java > d8a51a6 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/NotificationHandlerInvoker.java > 317c97b > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java > e7b6d17 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java > 4b31b0b > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java > c1eafe4 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/appender/RollingFileWithoutDeleteAppender.java > 7ca5813 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntity.java > f7edeb1 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryGMPrivilege.java > 266f349 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java > 1c68a0f > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FixedJsonInstanceSerializer.java > 6eb36a1 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HAContext.java > ada6308 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 6798f2f > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/ServiceManager.java > 0e3c0bb > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/ServiceRegister.java > 1e17f9a > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java > cbc0aaf > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java > 74f379a > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java > 4f8c834 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryWebServer.java > 43f28ea > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaHelper.java > e3e04f1 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaTool.java > 11b2ed2 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HAClientInvocationHandler.java > 377e934 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java > 1e7a789 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java > 26a32e4 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java > 5847cb5 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStoreToAuthorizable.java > 922cbc2 > > sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFiles.java > 4e5d4b9 > > sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java > 1b83c0d > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/SecureRequestHandlerUtil.java > 94341b3 > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/admin/SecureAdminHandlers.java > 88016ea > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/admin/SecureCollectionsHandler.java > 15a6ba0 > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/admin/SecureInfoHandler.java > 90b898b > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/QueryDocAuthorizationComponent.java > 371787d > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/QueryIndexAuthorizationComponent.java > 8f68f40 > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/sentry/RollingFileWithoutDeleteAppender.java > ec26ef3 > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/sentry/SentryIndexAuthorizationSingleton.java > 185884b > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/update/processor/UpdateIndexAuthorizationProcessor.java > 5e60645 > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/update/processor/UpdateIndexAuthorizationProcessorFactory.java > 945dbc4 > > sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/handler/TestSecureReplicationHandler.java > 9387677 > > sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/handler/admin/SecureAdminHandlersTest.java > 3cb2597 > > sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/handler/admin/SecureCoreAdminHandlerTest.java > 2a19902 > > sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/handler/admin/SecureInfoHandlerTest.java > 7221fa0 > > Diff: https://reviews.apache.org/r/41349/diff/ > > > Testing > ------- > > No tests required. > > > Thanks, > > Colm O hEigeartaigh > >
