accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Corey Nolet" <cjno...@gmail.com>
Subject Re: Review Request 29502: ACCUMULO-3458 Adding scan authorizations to IteratorEnvironment
Date Wed, 31 Dec 2014 13:46:35 GMT


> On Dec. 31, 2014, 4:30 a.m., Josh Elser wrote:
> > core/src/main/java/org/apache/accumulo/core/client/ClientSideIteratorScanner.java,
line 197
> > <https://reviews.apache.org/r/29502/diff/1/?file=804415#file804415line197>
> >
> >     Can't you pull this from the Scanner?

I didn't see a good way to get this info from the scanner. The more I think about this- a
simple getter on the scanner would be massively useful.


> On Dec. 31, 2014, 4:30 a.m., Josh Elser wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/TabletIteratorEnvironment.java,
line 55
> > <https://reviews.apache.org/r/29502/diff/1/?file=804426#file804426line55>
> >
> >     It looks like TabletIteratorEnvironment is used for minor compactions. Isn't
always setting `Authorizations.EMPTY` a little misleading? Is there something more representative
of having all auths we could do here? Maybe extra documentation is enough? Could also throw
UnsupportedOperationException or similar when the IteratorScope is something that isn't SCAN?

Good point! This should definitely be documented as a scan-time only operation. I'm on the
fence about throwing an exception- I think I could go either way on that.


> On Dec. 31, 2014, 4:30 a.m., Josh Elser wrote:
> > test/src/test/java/org/apache/accumulo/test/ScanIteratorIT.java, line 54
> > <https://reviews.apache.org/r/29502/diff/1/?file=804430#file804430line54>
> >
> >     Please create a user, assign it the auths you need, and then remove the user
after the test.
> >     
> >     If this test is run against a standalone instance, it should try to leave the
system in the same state the test started in.

You know I was thinking about this when I was coding the test and totally forgot to change
it before I created the patch.


- Corey


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


On Dec. 31, 2014, 1:46 p.m., Corey Nolet wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29502/
> -----------------------------------------------------------
> 
> (Updated Dec. 31, 2014, 1:46 p.m.)
> 
> 
> Review request for accumulo, Christopher Tubbs, Eric Newton, Josh Elser, and kturner.
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> ACCUMULO-3458 Propagating scan-time authorizations through the IteratorEnvironment so
that scan-time iterators can use them.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/client/ClientSideIteratorScanner.java 4903656

>   core/src/main/java/org/apache/accumulo/core/client/ScannerBase.java 335b63a 
>   core/src/main/java/org/apache/accumulo/core/client/impl/OfflineScanner.java 2552682

>   core/src/main/java/org/apache/accumulo/core/client/impl/ScannerImpl.java 666a8af 
>   core/src/main/java/org/apache/accumulo/core/client/impl/ScannerOptions.java 9726266

>   core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchReader.java
2a79f05 
>   core/src/main/java/org/apache/accumulo/core/client/mock/MockScannerBase.java 72cb863

>   core/src/main/java/org/apache/accumulo/core/iterators/IteratorEnvironment.java 9e20cb1

>   core/src/main/java/org/apache/accumulo/core/iterators/system/VisibilityFilter.java
15c33fa 
>   core/src/test/java/org/apache/accumulo/core/iterators/DefaultIteratorEnvironment.java
94da7b5 
>   core/src/test/java/org/apache/accumulo/core/iterators/FirstEntryInRowIteratorTest.java
fa46360 
>   core/src/test/java/org/apache/accumulo/core/iterators/user/RowDeletingIteratorTest.java
4521e55 
>   core/src/test/java/org/apache/accumulo/core/iterators/user/TransformingIteratorTest.java
4cebab7 
>   server/base/src/test/java/org/apache/accumulo/server/iterators/MetadataBulkLoadFilterTest.java
4a45e99 
>   server/base/src/test/java/org/apache/accumulo/server/replication/StatusCombinerTest.java
a9801b0 
>   server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/trace/NullScanner.java
bf35557 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletIteratorEnvironment.java
d1fece5 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Compactor.java 869cc33

>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/ScanDataSource.java
fe4b16b 
>   test/src/main/java/org/apache/accumulo/test/functional/AuthsIterator.java PRE-CREATION

>   test/src/test/java/org/apache/accumulo/test/ScanIteratorIT.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29502/diff/
> 
> 
> Testing
> -------
> 
> Wrote an integration test to verify that ScanDataSource is actually setting the authorizations
on the IteratorEnvironment
> 
> 
> Thanks,
> 
> Corey Nolet
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message