You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Colm O hEigeartaigh <co...@apache.org> on 2016/09/13 15:57:53 UTC

Review Request 51848: Apply Checkstyle changes to the core

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51848/
-----------------------------------------------------------

Review request for sentry.


Bugs: SENTRY-1470
    https://issues.apache.org/jira/browse/SENTRY-1470


Repository: sentry


Description
-------

This task is to apply checkstyle fixes to the core. Once all modules are covered, the rules can be enabled by default.


Diffs
-----

  sentry-binding/sentry-binding-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaPrivilegeValidator.java ba66d43 
  sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java 3bc5b82 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/SentryMain.java d321531 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/SentryVersionInfo.java de77dc3 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/BitFieldAction.java 0f5b23b 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/Resource.java 3ce52e8 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryUserException.java 3b5beda 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/AuthorizationComponent.java e3f1f15 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PathUtils.java 40c9595 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PolicyFile.java a6ef0b3 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PolicyFileConstants.java 6b625ff 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PolicyFiles.java 4a632bc 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/validator/PrivilegeValidatorContext.java ccee977 
  sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/DBModelAuthorizables.java 7bc94c9 
  sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/HiveActionFactory.java ad7e1c9 
  sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/HivePrivilegeModel.java 231acca 
  sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/validator/AbstractDBPrivilegeValidator.java fa28716 
  sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/validator/DatabaseMustMatch.java 4276667 
  sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/validator/DatabaseRequiredInPrivilege.java fed3038 
  sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/validator/ServerNameMustMatch.java c79a8bf 
  sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/validator/ServersAllIsInvalid.java e3f5a3a 
  sentry-core/sentry-core-model-db/src/test/java/org/apache/sentry/core/db/TestURI.java 40b60f7 
  sentry-core/sentry-core-model-indexer/src/main/java/org/apache/sentry/core/model/indexer/IndexerActionFactory.java 3ca85bc 
  sentry-core/sentry-core-model-indexer/src/main/java/org/apache/sentry/core/model/indexer/IndexerModelAuthorizables.java 414df68 
  sentry-core/sentry-core-model-indexer/src/main/java/org/apache/sentry/core/model/indexer/IndexerPrivilegeModel.java 6951513 
  sentry-core/sentry-core-model-indexer/src/main/java/org/apache/sentry/core/model/indexer/validator/AbstractIndexerPrivilegeValidator.java c73fc3c 
  sentry-core/sentry-core-model-indexer/src/main/java/org/apache/sentry/core/model/indexer/validator/IndexerRequiredInPrivilege.java 013c572 
  sentry-core/sentry-core-model-indexer/src/test/java/org/apache/sentry/core/indexer/TestIndexerBitFieldAction.java 532f9ec 
  sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaActionConstant.java 17d7fb7 
  sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaActionFactory.java a1fec1f 
  sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaModelAuthorizables.java 45a1148 
  sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaPrivilegeModel.java e460874 
  sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/validator/KafkaPrivilegeValidator.java 5c45865 
  sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/Field.java 2dd9065 
  sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/SearchActionFactory.java 3f10726 
  sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/SearchModelAuthorizables.java 2b190e5 
  sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/SearchPrivilegeModel.java 9429a25 
  sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/validator/AbstractSearchPrivilegeValidator.java c06131c 
  sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/validator/CollectionRequiredInPrivilege.java 93b3861 
  sentry-core/sentry-core-model-search/src/test/java/org/apache/sentry/core/search/TestSearchBitFieldAction.java 0056f40 
  sentry-core/sentry-core-model-sqoop/src/main/java/org/apache/sentry/core/model/sqoop/SqoopActionFactory.java e7ba5f1 
  sentry-core/sentry-core-model-sqoop/src/main/java/org/apache/sentry/core/model/sqoop/SqoopModelAuthorizables.java 3bb9a19 
  sentry-core/sentry-core-model-sqoop/src/main/java/org/apache/sentry/core/model/sqoop/SqoopPrivilegeModel.java 4bd8f94 
  sentry-core/sentry-core-model-sqoop/src/main/java/org/apache/sentry/core/model/sqoop/validator/ServerNameRequiredMatch.java 67347bc 
  sentry-core/sentry-core-model-sqoop/src/test/java/org/apache/sentry/core/model/sqoop/TestSqoopAction.java 9c86158 
  sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellKafka.java 7db5426 

Diff: https://reviews.apache.org/r/51848/diff/


Testing
-------


Thanks,

Colm O hEigeartaigh


Re: Review Request 51848: Apply Checkstyle changes to the core

Posted by Colm O hEigeartaigh <co...@apache.org>.

> On Oct. 17, 2016, 8:34 p.m., Alexander Kolbasov wrote:
> > My big concern with these changes is that they cause more deviation between trunk and sentry-ha branch where there are a lot of changes happening. The style changes are very useful but they cause quite complicated merges. I think that we should consider delaying these changes until we merge sentry HA branch into trunk.

OK fair enough. What's the timeline on merging the Sentry HA branck to trunk?


- Colm


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51848/#review152947
-----------------------------------------------------------


On Sept. 13, 2016, 3:57 p.m., Colm O hEigeartaigh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51848/
> -----------------------------------------------------------
> 
> (Updated Sept. 13, 2016, 3:57 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: SENTRY-1470
>     https://issues.apache.org/jira/browse/SENTRY-1470
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This task is to apply checkstyle fixes to the core. Once all modules are covered, the rules can be enabled by default.
> 
> 
> Diffs
> -----
> 
>   sentry-binding/sentry-binding-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaPrivilegeValidator.java ba66d43 
>   sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java 3bc5b82 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/SentryMain.java d321531 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/SentryVersionInfo.java de77dc3 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/BitFieldAction.java 0f5b23b 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/Resource.java 3ce52e8 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryUserException.java 3b5beda 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/AuthorizationComponent.java e3f1f15 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PathUtils.java 40c9595 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PolicyFile.java a6ef0b3 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PolicyFileConstants.java 6b625ff 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PolicyFiles.java 4a632bc 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/validator/PrivilegeValidatorContext.java ccee977 
>   sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/DBModelAuthorizables.java 7bc94c9 
>   sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/HiveActionFactory.java ad7e1c9 
>   sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/HivePrivilegeModel.java 231acca 
>   sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/validator/AbstractDBPrivilegeValidator.java fa28716 
>   sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/validator/DatabaseMustMatch.java 4276667 
>   sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/validator/DatabaseRequiredInPrivilege.java fed3038 
>   sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/validator/ServerNameMustMatch.java c79a8bf 
>   sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/validator/ServersAllIsInvalid.java e3f5a3a 
>   sentry-core/sentry-core-model-db/src/test/java/org/apache/sentry/core/db/TestURI.java 40b60f7 
>   sentry-core/sentry-core-model-indexer/src/main/java/org/apache/sentry/core/model/indexer/IndexerActionFactory.java 3ca85bc 
>   sentry-core/sentry-core-model-indexer/src/main/java/org/apache/sentry/core/model/indexer/IndexerModelAuthorizables.java 414df68 
>   sentry-core/sentry-core-model-indexer/src/main/java/org/apache/sentry/core/model/indexer/IndexerPrivilegeModel.java 6951513 
>   sentry-core/sentry-core-model-indexer/src/main/java/org/apache/sentry/core/model/indexer/validator/AbstractIndexerPrivilegeValidator.java c73fc3c 
>   sentry-core/sentry-core-model-indexer/src/main/java/org/apache/sentry/core/model/indexer/validator/IndexerRequiredInPrivilege.java 013c572 
>   sentry-core/sentry-core-model-indexer/src/test/java/org/apache/sentry/core/indexer/TestIndexerBitFieldAction.java 532f9ec 
>   sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaActionConstant.java 17d7fb7 
>   sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaActionFactory.java a1fec1f 
>   sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaModelAuthorizables.java 45a1148 
>   sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaPrivilegeModel.java e460874 
>   sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/validator/KafkaPrivilegeValidator.java 5c45865 
>   sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/Field.java 2dd9065 
>   sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/SearchActionFactory.java 3f10726 
>   sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/SearchModelAuthorizables.java 2b190e5 
>   sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/SearchPrivilegeModel.java 9429a25 
>   sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/validator/AbstractSearchPrivilegeValidator.java c06131c 
>   sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/validator/CollectionRequiredInPrivilege.java 93b3861 
>   sentry-core/sentry-core-model-search/src/test/java/org/apache/sentry/core/search/TestSearchBitFieldAction.java 0056f40 
>   sentry-core/sentry-core-model-sqoop/src/main/java/org/apache/sentry/core/model/sqoop/SqoopActionFactory.java e7ba5f1 
>   sentry-core/sentry-core-model-sqoop/src/main/java/org/apache/sentry/core/model/sqoop/SqoopModelAuthorizables.java 3bb9a19 
>   sentry-core/sentry-core-model-sqoop/src/main/java/org/apache/sentry/core/model/sqoop/SqoopPrivilegeModel.java 4bd8f94 
>   sentry-core/sentry-core-model-sqoop/src/main/java/org/apache/sentry/core/model/sqoop/validator/ServerNameRequiredMatch.java 67347bc 
>   sentry-core/sentry-core-model-sqoop/src/test/java/org/apache/sentry/core/model/sqoop/TestSqoopAction.java 9c86158 
>   sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellKafka.java 7db5426 
> 
> Diff: https://reviews.apache.org/r/51848/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Colm O hEigeartaigh
> 
>


Re: Review Request 51848: Apply Checkstyle changes to the core

Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51848/#review152947
-----------------------------------------------------------



My big concern with these changes is that they cause more deviation between trunk and sentry-ha branch where there are a lot of changes happening. The style changes are very useful but they cause quite complicated merges. I think that we should consider delaying these changes until we merge sentry HA branch into trunk.

- Alexander Kolbasov


On Sept. 13, 2016, 3:57 p.m., Colm O hEigeartaigh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51848/
> -----------------------------------------------------------
> 
> (Updated Sept. 13, 2016, 3:57 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: SENTRY-1470
>     https://issues.apache.org/jira/browse/SENTRY-1470
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This task is to apply checkstyle fixes to the core. Once all modules are covered, the rules can be enabled by default.
> 
> 
> Diffs
> -----
> 
>   sentry-binding/sentry-binding-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaPrivilegeValidator.java ba66d43 
>   sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java 3bc5b82 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/SentryMain.java d321531 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/SentryVersionInfo.java de77dc3 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/BitFieldAction.java 0f5b23b 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/Resource.java 3ce52e8 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryUserException.java 3b5beda 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/AuthorizationComponent.java e3f1f15 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PathUtils.java 40c9595 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PolicyFile.java a6ef0b3 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PolicyFileConstants.java 6b625ff 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PolicyFiles.java 4a632bc 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/validator/PrivilegeValidatorContext.java ccee977 
>   sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/DBModelAuthorizables.java 7bc94c9 
>   sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/HiveActionFactory.java ad7e1c9 
>   sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/HivePrivilegeModel.java 231acca 
>   sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/validator/AbstractDBPrivilegeValidator.java fa28716 
>   sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/validator/DatabaseMustMatch.java 4276667 
>   sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/validator/DatabaseRequiredInPrivilege.java fed3038 
>   sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/validator/ServerNameMustMatch.java c79a8bf 
>   sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/validator/ServersAllIsInvalid.java e3f5a3a 
>   sentry-core/sentry-core-model-db/src/test/java/org/apache/sentry/core/db/TestURI.java 40b60f7 
>   sentry-core/sentry-core-model-indexer/src/main/java/org/apache/sentry/core/model/indexer/IndexerActionFactory.java 3ca85bc 
>   sentry-core/sentry-core-model-indexer/src/main/java/org/apache/sentry/core/model/indexer/IndexerModelAuthorizables.java 414df68 
>   sentry-core/sentry-core-model-indexer/src/main/java/org/apache/sentry/core/model/indexer/IndexerPrivilegeModel.java 6951513 
>   sentry-core/sentry-core-model-indexer/src/main/java/org/apache/sentry/core/model/indexer/validator/AbstractIndexerPrivilegeValidator.java c73fc3c 
>   sentry-core/sentry-core-model-indexer/src/main/java/org/apache/sentry/core/model/indexer/validator/IndexerRequiredInPrivilege.java 013c572 
>   sentry-core/sentry-core-model-indexer/src/test/java/org/apache/sentry/core/indexer/TestIndexerBitFieldAction.java 532f9ec 
>   sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaActionConstant.java 17d7fb7 
>   sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaActionFactory.java a1fec1f 
>   sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaModelAuthorizables.java 45a1148 
>   sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaPrivilegeModel.java e460874 
>   sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/validator/KafkaPrivilegeValidator.java 5c45865 
>   sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/Field.java 2dd9065 
>   sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/SearchActionFactory.java 3f10726 
>   sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/SearchModelAuthorizables.java 2b190e5 
>   sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/SearchPrivilegeModel.java 9429a25 
>   sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/validator/AbstractSearchPrivilegeValidator.java c06131c 
>   sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/validator/CollectionRequiredInPrivilege.java 93b3861 
>   sentry-core/sentry-core-model-search/src/test/java/org/apache/sentry/core/search/TestSearchBitFieldAction.java 0056f40 
>   sentry-core/sentry-core-model-sqoop/src/main/java/org/apache/sentry/core/model/sqoop/SqoopActionFactory.java e7ba5f1 
>   sentry-core/sentry-core-model-sqoop/src/main/java/org/apache/sentry/core/model/sqoop/SqoopModelAuthorizables.java 3bb9a19 
>   sentry-core/sentry-core-model-sqoop/src/main/java/org/apache/sentry/core/model/sqoop/SqoopPrivilegeModel.java 4bd8f94 
>   sentry-core/sentry-core-model-sqoop/src/main/java/org/apache/sentry/core/model/sqoop/validator/ServerNameRequiredMatch.java 67347bc 
>   sentry-core/sentry-core-model-sqoop/src/test/java/org/apache/sentry/core/model/sqoop/TestSqoopAction.java 9c86158 
>   sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellKafka.java 7db5426 
> 
> Diff: https://reviews.apache.org/r/51848/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Colm O hEigeartaigh
> 
>