> 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
> 
>

Reply via email to