You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Arjun Mishra via Review Board <no...@reviews.apache.org> on 2018/06/21 21:18:57 UTC

Review Request 67695: SENTRY-2277: Add to SentryStore testURI test case testing with multiple URI privileges

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

Review request for sentry, kalyan kumar kalvagadda and Sergio Pena.


Repository: sentry


Description
-------

This is a follow up to SENTRY-2231 to add test cases to validate


Diffs
-----

  sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 51048bc29 


Diff: https://reviews.apache.org/r/67695/diff/1/


Testing
-------

$  mvn -f sentry-service/sentry-service-server/pom.xml test -Dtest=TestSentryStore


Thanks,

Arjun Mishra


Re: Review Request 67695: SENTRY-2277: Add to SentryStore testURI test case testing with multiple URI privileges

Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67695/
-----------------------------------------------------------

(Updated June 29, 2018, 8:53 p.m.)


Review request for sentry, kalyan kumar kalvagadda and Sergio Pena.


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


Repository: sentry


Description
-------

This is a follow up to SENTRY-2231 to add test cases to validate


Diffs
-----

  sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 51048bc29 


Diff: https://reviews.apache.org/r/67695/diff/1/


Testing
-------

$  mvn -f sentry-service/sentry-service-server/pom.xml test -Dtest=TestSentryStore


Thanks,

Arjun Mishra


Re: Review Request 67695: SENTRY-2277: Add to SentryStore testURI test case testing with multiple URI privileges

Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67695/#review205510
-----------------------------------------------------------


Ship it!




Ship It!

- Na Li


On June 21, 2018, 9:18 p.m., Arjun Mishra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67695/
> -----------------------------------------------------------
> 
> (Updated June 21, 2018, 9:18 p.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This is a follow up to SENTRY-2231 to add test cases to validate
> 
> 
> Diffs
> -----
> 
>   sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 51048bc29 
> 
> 
> Diff: https://reviews.apache.org/r/67695/diff/1/
> 
> 
> Testing
> -------
> 
> $  mvn -f sentry-service/sentry-service-server/pom.xml test -Dtest=TestSentryStore
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>


Re: Review Request 67695: SENTRY-2277: Add to SentryStore testURI test case testing with multiple URI privileges

Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67695/#review205507
-----------------------------------------------------------


Ship it!




Ship It!

- kalyan kumar kalvagadda


On June 21, 2018, 9:18 p.m., Arjun Mishra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67695/
> -----------------------------------------------------------
> 
> (Updated June 21, 2018, 9:18 p.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This is a follow up to SENTRY-2231 to add test cases to validate
> 
> 
> Diffs
> -----
> 
>   sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 51048bc29 
> 
> 
> Diff: https://reviews.apache.org/r/67695/diff/1/
> 
> 
> Testing
> -------
> 
> $  mvn -f sentry-service/sentry-service-server/pom.xml test -Dtest=TestSentryStore
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>


Re: Review Request 67695: SENTRY-2277: Add to SentryStore testURI test case testing with multiple URI privileges

Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.

> On June 21, 2018, 10:16 p.m., kalyan kumar kalvagadda wrote:
> > Arjun,
> > 
> > Can you explian what was wrong with old test case?

The test case has been changeed to include a new role, role2, and a new uri privilege granted to this role. If we were to set active role set as both role1 and role2, and set say authorizable to uri1, the old code would've broken the test as it would've returned 2 results (both URI privileges). assertTrue(privileges.size() == 1) would be FALSE. This is becuse it wasn't using the other condition check to compare URIs


- Arjun


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


On June 21, 2018, 9:18 p.m., Arjun Mishra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67695/
> -----------------------------------------------------------
> 
> (Updated June 21, 2018, 9:18 p.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This is a follow up to SENTRY-2231 to add test cases to validate
> 
> 
> Diffs
> -----
> 
>   sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 51048bc29 
> 
> 
> Diff: https://reviews.apache.org/r/67695/diff/1/
> 
> 
> Testing
> -------
> 
> $  mvn -f sentry-service/sentry-service-server/pom.xml test -Dtest=TestSentryStore
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>


Re: Review Request 67695: SENTRY-2277: Add to SentryStore testURI test case testing with multiple URI privileges

Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67695/#review205212
-----------------------------------------------------------



Arjun,

Can you explian what was wrong with old test case?

- kalyan kumar kalvagadda


On June 21, 2018, 9:18 p.m., Arjun Mishra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67695/
> -----------------------------------------------------------
> 
> (Updated June 21, 2018, 9:18 p.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This is a follow up to SENTRY-2231 to add test cases to validate
> 
> 
> Diffs
> -----
> 
>   sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 51048bc29 
> 
> 
> Diff: https://reviews.apache.org/r/67695/diff/1/
> 
> 
> Testing
> -------
> 
> $  mvn -f sentry-service/sentry-service-server/pom.xml test -Dtest=TestSentryStore
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>