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 Tue, 06 Jan 2015 15:43:07 GMT


> On Jan. 5, 2015, 9:09 p.m., Christopher Tubbs wrote:
> > core/src/test/java/org/apache/accumulo/core/iterators/FirstEntryInRowIteratorTest.java,
line 63
> > <https://reviews.apache.org/r/29502/diff/2/?file=804705#file804705line63>
> >
> >     Should this be Authorizations.EMPTY? Or should it have a default implementation
on WrappingIterator which calls source.getAuthorizations()?
> 
> Christopher Tubbs wrote:
>     make that `getSource().getAuthorizations()`

Specific to this test I returned null because all the other getters (other than what was being
explicitly tested) were returning null. Were you thinking WrappingIterator should also provide
a getAuthorizations() method?


> On Jan. 5, 2015, 9:09 p.m., Christopher Tubbs wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/TabletIteratorEnvironment.java,
line 46
> > <https://reviews.apache.org/r/29502/diff/2/?file=804711#file804711line46>
> >
> >     I wonder if there's a better way to provide environment options, like this and
others, at specific scopes. Maybe use some dependency injection, with annotations, like Servlet
@Context or JUnit @Rule: @ScanContext Authorizations auths; (throw error if type is not appropriate
for context during injection).

This feature would be pretty neat. Were you thinking this would extend past just the IteratorEnvironment
into other places? Any other fields you can think of that would benefit from this change other
than Authorizations?


- Corey


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


On Dec. 31, 2014, 3:40 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, 3:40 p.m.)
> 
> 
> Review request for accumulo, Christopher Tubbs, Eric Newton, Josh Elser, and kturner.
> 
> 
> Bugs: ACCUMULO-3458
>     https://issues.apache.org/jira/browse/ACCUMULO-3458
> 
> 
> 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