You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Vamsee Yarlagadda <va...@cloudera.com> on 2014/01/09 23:05:31 UTC
Review Request 16766: SENTRY-84: Solr Admin/Collection checks
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16766/
-----------------------------------------------------------
Review request for sentry and Gregory Chanan.
Bugs: SENTRY-84
https://issues.apache.org/jira/browse/SENTRY-84
Repository: sentry
Description
-------
Tests for checking collection/admin commands with Sentry.
- .ini file has been pre-populated with all possible privileges.
- Validates - CREATE,DELETE,RELOAD,SPLITSHARD,CREATEALIAS,DELETEALIAS,DELETESHARD
This single test case is taking a long time to complete. Is it better to split this class into smaller chunks?
Diffs
-----
sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/AbstractSolrSentryTestBase.java fcbc67c05e90335c41b0abc93bbe43b67480f27a
sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestAdminOperations.java PRE-CREATION
sentry-tests/sentry-tests-solr/src/test/resources/solr/sentry/test-authz-provider.ini f988eaebea3f0bb4e67dfab179fdaff298ac55ca
Diff: https://reviews.apache.org/r/16766/diff/
Testing
-------
Ran the tests in the Mini DFS cluster and they passed without a problem. But takes long time to complete
Thanks,
Vamsee Yarlagadda
Re: Review Request 16766: SENTRY-84: Solr Admin/Collection checks
Posted by Vamsee Yarlagadda <va...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16766/
-----------------------------------------------------------
(Updated Jan. 11, 2014, 12:29 a.m.)
Review request for sentry and Gregory Chanan.
Bugs: SENTRY-84
https://issues.apache.org/jira/browse/SENTRY-84
Repository: sentry
Description
-------
Tests for checking collection/admin commands with Sentry.
- .ini file has been pre-populated with all possible privileges.
- Validates - CREATE,DELETE,RELOAD,SPLITSHARD,CREATEALIAS,DELETEALIAS,DELETESHARD
This single test case is taking a long time to complete. Is it better to split this class into smaller chunks?
Diffs (updated)
-----
sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/AbstractSolrSentryTestBase.java fcbc67c05e90335c41b0abc93bbe43b67480f27a
sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestCollAdminAliasOperations.java PRE-CREATION
sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestCollAdminCoreOperations.java PRE-CREATION
sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestCollAdminShardOperations.java PRE-CREATION
sentry-tests/sentry-tests-solr/src/test/resources/solr/sentry/test-authz-provider.ini f988eaebea3f0bb4e67dfab179fdaff298ac55ca
Diff: https://reviews.apache.org/r/16766/diff/
Testing
-------
Ran the tests in the Mini DFS cluster and they passed without a problem. But takes long time to complete
Thanks,
Vamsee Yarlagadda
Re: Review Request 16766: SENTRY-84: Solr Admin/Collection checks
Posted by Vamsee Yarlagadda <va...@cloudera.com>.
> On Jan. 10, 2014, 4:04 a.m., Gregory Chanan wrote:
> > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/AbstractSolrSentryTestBase.java, line 653
> > <https://reviews.apache.org/r/16766/diff/2/?file=419820#file419820line653>
> >
> > is there a reason we need to ignoreErrors? Maybe at least document it in the javadoc?
Added info to the javadoc for "verifyCollectionAdminOpPass"
> On Jan. 10, 2014, 4:04 a.m., Gregory Chanan wrote:
> > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestAdminOperations.java, line 67
> > <https://reviews.apache.org/r/16766/diff/2/?file=419821#file419821line67>
> >
> > can we just iterate over the collection actions?
I changed the implementation to split this class into sub classes as running everything in one class is resulting in timeout issues.
> On Jan. 10, 2014, 4:04 a.m., Gregory Chanan wrote:
> > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestAdminOperations.java, line 151
> > <https://reviews.apache.org/r/16766/diff/2/?file=419821#file419821line151>
> >
> > same here, why is this done twice?
In order to delete an alias, we need to have an alias in the first place. This step would setup the environment for this.
> On Jan. 10, 2014, 4:04 a.m., Gregory Chanan wrote:
> > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestAdminOperations.java, line 104
> > <https://reviews.apache.org/r/16766/diff/2/?file=419821#file419821line104>
> >
> > is there a reason this is done twice?
In order to delete an alias, we need to have an alias in the first place. This step would setup the environment for this.
- Vamsee
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16766/#review31508
-----------------------------------------------------------
On Jan. 9, 2014, 10:05 p.m., Vamsee Yarlagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16766/
> -----------------------------------------------------------
>
> (Updated Jan. 9, 2014, 10:05 p.m.)
>
>
> Review request for sentry and Gregory Chanan.
>
>
> Bugs: SENTRY-84
> https://issues.apache.org/jira/browse/SENTRY-84
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Tests for checking collection/admin commands with Sentry.
> - .ini file has been pre-populated with all possible privileges.
> - Validates - CREATE,DELETE,RELOAD,SPLITSHARD,CREATEALIAS,DELETEALIAS,DELETESHARD
>
> This single test case is taking a long time to complete. Is it better to split this class into smaller chunks?
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/AbstractSolrSentryTestBase.java fcbc67c05e90335c41b0abc93bbe43b67480f27a
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestAdminOperations.java PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/solr/sentry/test-authz-provider.ini f988eaebea3f0bb4e67dfab179fdaff298ac55ca
>
> Diff: https://reviews.apache.org/r/16766/diff/
>
>
> Testing
> -------
>
> Ran the tests in the Mini DFS cluster and they passed without a problem. But takes long time to complete
>
>
> Thanks,
>
> Vamsee Yarlagadda
>
>
Re: Review Request 16766: SENTRY-84: Solr Admin/Collection checks
Posted by Gregory Chanan <gc...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16766/#review31508
-----------------------------------------------------------
Looks really nice.
sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/AbstractSolrSentryTestBase.java
<https://reviews.apache.org/r/16766/#comment60009>
hmm...i know you want to capitalize the sentence, but Boolean is a different type than boolean. I'd just not capitalize.
sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/AbstractSolrSentryTestBase.java
<https://reviews.apache.org/r/16766/#comment60012>
maybe we can remove some duplicate code by making a function that retuns a solrServer given the adminOp and collectionName and use it in verifyCollectionAdminOpFail.
sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/AbstractSolrSentryTestBase.java
<https://reviews.apache.org/r/16766/#comment60011>
can we make multiple underlying collections just for completeness?
sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/AbstractSolrSentryTestBase.java
<https://reviews.apache.org/r/16766/#comment60010>
it may be defined, maybe "not supported" is better.
sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/AbstractSolrSentryTestBase.java
<https://reviews.apache.org/r/16766/#comment60013>
Cool!
sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/AbstractSolrSentryTestBase.java
<https://reviews.apache.org/r/16766/#comment60014>
is there a reason we need to ignoreErrors? Maybe at least document it in the javadoc?
sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestAdminOperations.java
<https://reviews.apache.org/r/16766/#comment60017>
I like the thoroughness of this...how long is the runtime? Maybe we should have a mvn profile that runs a shorter version? (or maybe make the default the shorter version and the profile version the longer one? What do you think?
sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestAdminOperations.java
<https://reviews.apache.org/r/16766/#comment60020>
can we just iterate over the collection actions?
sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestAdminOperations.java
<https://reviews.apache.org/r/16766/#comment60018>
is there a reason this is done twice?
sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestAdminOperations.java
<https://reviews.apache.org/r/16766/#comment60016>
can we check using the underlying aliases? Maybe in a different test?
sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestAdminOperations.java
<https://reviews.apache.org/r/16766/#comment60019>
same here, why is this done twice?
sentry-tests/sentry-tests-solr/src/test/resources/solr/sentry/test-authz-provider.ini
<https://reviews.apache.org/r/16766/#comment60015>
I'd prefer generating these, but no big deal.
- Gregory Chanan
On Jan. 9, 2014, 10:05 p.m., Vamsee Yarlagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16766/
> -----------------------------------------------------------
>
> (Updated Jan. 9, 2014, 10:05 p.m.)
>
>
> Review request for sentry and Gregory Chanan.
>
>
> Bugs: SENTRY-84
> https://issues.apache.org/jira/browse/SENTRY-84
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Tests for checking collection/admin commands with Sentry.
> - .ini file has been pre-populated with all possible privileges.
> - Validates - CREATE,DELETE,RELOAD,SPLITSHARD,CREATEALIAS,DELETEALIAS,DELETESHARD
>
> This single test case is taking a long time to complete. Is it better to split this class into smaller chunks?
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/AbstractSolrSentryTestBase.java fcbc67c05e90335c41b0abc93bbe43b67480f27a
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestAdminOperations.java PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/solr/sentry/test-authz-provider.ini f988eaebea3f0bb4e67dfab179fdaff298ac55ca
>
> Diff: https://reviews.apache.org/r/16766/diff/
>
>
> Testing
> -------
>
> Ran the tests in the Mini DFS cluster and they passed without a problem. But takes long time to complete
>
>
> Thanks,
>
> Vamsee Yarlagadda
>
>