> On Dec. 15, 2015, 4:30 a.m., Hao Hao wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java, > > line 34 > > <https://reviews.apache.org/r/41349/diff/1/?file=1162360#file1162360line34> > > > > Should we remove this line?
The problem is that the serverPrivilege variable is not used, PMD throws an error as a result. Should we be inserting it in the hiveAuthzStmtPrivMap as per the other HiveAuthzPrivileges? > On Dec. 15, 2015, 4:30 a.m., Hao Hao wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java, > > line 267 > > <https://reviews.apache.org/r/41349/diff/1/?file=1162354#file1162354line267> > > > > Feel with (), the code is more readable here. PMD will throw error on > > this one? Yep, PMD doesn't like any unnecessary brackets. If you would like I could leave the brackets in and add a //NOPMD comment for this line? - Colm ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41349/#review110402 ----------------------------------------------------------- On Dec. 14, 2015, 2:41 p.m., Colm O hEigeartaigh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41349/ > ----------------------------------------------------------- > > (Updated Dec. 14, 2015, 2:41 p.m.) > > > Review request for sentry. > > > Repository: sentry > > > Description > ------- > > Some query points where I inserted a "//NOPMD" comment to avoid breaking the > existing source > > 1) HiveAuthzBindingHook contains some empty exceptions. Should the "//NOPMD" > comment remain here or does it make sense to at least log the exception? > > 2) SentryHiveAuthorizationTaskFactoryImpl contains a constructor with two > unused parameters. Should this constructor be removed? > > 3) SentryHiveMetaStoreClient + SentryMetaStoreFilterHook have a HiveConf > class variable that isn't used. Remove? > > 4) MetastorePlugin has a ExecutorService class variable that isn't used. > Remove? > > 5) SimpleCacheProviderBackend + SentryGenericProviderBackend has a > constructor with two unused parameters. Remove? > > 6) The "resourcePath" parameters is not used in the SimpleDBProviderBackend > constructor. Remove? > > 7) MSentryGMPrivilege has a bunch of class variables that are not used. > Remove? > > > 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 > >
