You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Na Li <li...@cloudera.com> on 2017/06/05 23:10:38 UTC
Review Request 59820: SENTRY-1792: Too many full snapshot for
Permission change request
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59820/
-----------------------------------------------------------
Review request for sentry and Alexander Kolbasov.
Repository: sentry
Description
-------
extract continuous list from noncontinuous list
Diffs
-----
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 37eb0b2
Diff: https://reviews.apache.org/r/59820/diff/1/
Testing
-------
test on Vamsee's cluster
Thanks,
Na Li
Re: Review Request 59820: SENTRY-1792: Too many full snapshot for
Permission change request
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59820/#review177098
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 3776 (patched)
<https://reviews.apache.org/r/59820/#comment250646>
Please provide documentation for this method.
Note that changes may be unsorted, so this may not work.
Also, you may consider having a function that gets a list of IDs and returns a continuous initial segment and put it in some utility class rather then here.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 3797 (patched)
<https://reviews.apache.org/r/59820/#comment250647>
1) Please provide documentation
2) This belongs t an utility class - it has nothing to do with changes and can work on any list of IDs
3) It assumes that changes is sorted which isn't guaranteed.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 3815 (patched)
<https://reviews.apache.org/r/59820/#comment250648>
it isn't a good idea to mix string builder with string concatenation - it is better to use string joiner to combine elements.
- Alexander Kolbasov
On June 5, 2017, 11:10 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59820/
> -----------------------------------------------------------
>
> (Updated June 5, 2017, 11:10 p.m.)
>
>
> Review request for sentry and Alexander Kolbasov.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> extract continuous list from noncontinuous list
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 37eb0b2
>
>
> Diff: https://reviews.apache.org/r/59820/diff/1/
>
>
> Testing
> -------
>
> test on Vamsee's cluster
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 59820: SENTRY-1792: Too many full snapshot for
Permission change request
Posted by Vamsee Yarlagadda <va...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59820/#review177135
-----------------------------------------------------------
Fixed the code review comments at: https://reviews.apache.org/r/59869/
- Vamsee Yarlagadda
On June 5, 2017, 11:10 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59820/
> -----------------------------------------------------------
>
> (Updated June 5, 2017, 11:10 p.m.)
>
>
> Review request for sentry and Alexander Kolbasov.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> extract continuous list from noncontinuous list
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 37eb0b2
>
>
> Diff: https://reviews.apache.org/r/59820/diff/1/
>
>
> Testing
> -------
>
> test on Vamsee's cluster
>
>
> Thanks,
>
> Na Li
>
>