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 Thu, 08 Jan 2015 00:50:51 GMT


> On Jan. 1, 2015, 12:36 a.m., kturner wrote:
> > core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchReader.java,
line 78
> > <https://reviews.apache.org/r/29502/diff/2/?file=804700#file804700line78>
> >
> >     was putting @Override on same line as method decleration intentional?
> 
> Christopher Tubbs wrote:
>     Probably best to just format and organize imports for all the changed files. I noticed
a lot of other formatting issues, too.
> 
> Corey Nolet wrote:
>     Not sure why intelli-j defaults to this behavior but it's fixed.
> 
> Christopher Tubbs wrote:
>     Import order is something that our formatting standards don't even address, I just
noticed the change and thought it unusual.
> 
> Corey Nolet wrote:
>     This is something we worked out on Fluo early on and I believe the static changing
from the top of the imports to the bottom was a result of that- though I'm surprised, unless
Keith has multiple profiles for his import orders, why we wouldn't have noticed this sooner
in his patches.
>     
>     See https://github.com/fluo-io/fluo/wiki/Contributing#coding-guidelines
> 
> kturner wrote:
>     > unless Keith has multiple profiles for his import orders
>     
>     I use two eclipse workspaces w/ different config, one for Fluo and one for Accumulo.
 I try my best to avoid making changes unrelated to the task I am working.

Christopher, when I read over your comment early this morning, I read it as "Import order
is something that our formatting standards don't even address,  [maybe it's time we address
them.]" I referenced what we did on Fluo just to present the idea of putting those standards
on the site, not to say any they need to be changed.

> I use two eclipse workspaces w/ different config, one for Fluo and one for Accumulo
    
It makes sense now. I was wondering why the deltas were only happening for me. I just assumed
Mike and yourself were using the defaults that were already being used in Accumulo.


- Corey


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


On Jan. 6, 2015, 3:54 p.m., Corey Nolet wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29502/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2015, 3:54 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/test/java/org/apache/accumulo/core/client/impl/ScannerImplTest.java be4d467

>   core/src/test/java/org/apache/accumulo/core/client/impl/TabletServerBatchReaderTest.java
PRE-CREATION 
>   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