You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by ke...@deenlo.com on 2015/01/01 01:36:16 UTC
Re: Review Request 29502: ACCUMULO-3458 Adding scan authorizations to
IteratorEnvironment
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29502/#review66493
-----------------------------------------------------------
core/src/main/java/org/apache/accumulo/core/client/ScannerBase.java
<https://reviews.apache.org/r/29502/#comment110073>
Testing this method for scanner and batch scanner would be nice.
core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchReader.java
<https://reviews.apache.org/r/29502/#comment110070>
was putting @Override on same line as method decleration intentional?
test/src/test/java/org/apache/accumulo/test/ScanIteratorIT.java
<https://reviews.apache.org/r/29502/#comment110071>
could this be more strict, should the count be 1?
test/src/test/java/org/apache/accumulo/test/ScanIteratorIT.java
<https://reviews.apache.org/r/29502/#comment110072>
Two more test would be nice.
* Test the case where empty set of Auths is set on scanner, and verify AuthsIterator.FAIL is returned.
* Test case where set Auths("B") is set on scanner, and verify AuthsIterator.FAIL is returned.
The TransformingIterator could possible be modified to leverage this change. That could be done as a follow on issue.
- kturner
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
>
>
Re: Review Request 29502: ACCUMULO-3458 Adding scan authorizations to
IteratorEnvironment
Posted by Corey Nolet <cj...@gmail.com>.
> 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.
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
- 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
>
>
Re: Review Request 29502: ACCUMULO-3458 Adding scan authorizations to
IteratorEnvironment
Posted by Corey Nolet <cj...@gmail.com>.
> 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
>
>
Re: Review Request 29502: ACCUMULO-3458 Adding scan authorizations to
IteratorEnvironment
Posted by Christopher Tubbs <ct...@apache.org>.
> On Dec. 31, 2014, 7:36 p.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?
Probably best to just format and organize imports for all the changed files. I noticed a lot of other formatting issues, too.
- Christopher
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29502/#review66493
-----------------------------------------------------------
On Dec. 31, 2014, 10:40 a.m., Corey Nolet wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29502/
> -----------------------------------------------------------
>
> (Updated Dec. 31, 2014, 10:40 a.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
>
>
Re: Review Request 29502: ACCUMULO-3458 Adding scan authorizations to
IteratorEnvironment
Posted by Christopher Tubbs <ct...@apache.org>.
> On Dec. 31, 2014, 7:36 p.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.
Import order is something that our formatting standards don't even address, I just noticed the change and thought it unusual.
- Christopher
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29502/#review66493
-----------------------------------------------------------
On Jan. 6, 2015, 10:54 a.m., Corey Nolet wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29502/
> -----------------------------------------------------------
>
> (Updated Jan. 6, 2015, 10:54 a.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
>
>
Re: Review Request 29502: ACCUMULO-3458 Adding scan authorizations to
IteratorEnvironment
Posted by ke...@deenlo.com.
> 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
> 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.
- kturner
-----------------------------------------------------------
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
>
>
Re: Review Request 29502: ACCUMULO-3458 Adding scan authorizations to
IteratorEnvironment
Posted by Christopher Tubbs <ct...@apache.org>.
> On Dec. 31, 2014, 7:36 p.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.
>
> Corey Nolet wrote:
> 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, yes I guess I was hinting at an implied 'maybe it's time we address them'. But, I was also wanted to express that you shouldn't feel it necessary to make the change to comply with my preferences, since we haven't actually established a particular standard. In my initial comment, I didn't realize you weren't using Eclipse, but in my second comment, I was pointing out that even if you were using Eclipse with our formatter, it still wouldn't have (necessarily) addressed the import order if you had modified them from the defaults.
- Christopher
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29502/#review66493
-----------------------------------------------------------
On Jan. 6, 2015, 10:54 a.m., Corey Nolet wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29502/
> -----------------------------------------------------------
>
> (Updated Jan. 6, 2015, 10:54 a.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
>
>
Re: Review Request 29502: ACCUMULO-3458 Adding scan authorizations to
IteratorEnvironment
Posted by Corey Nolet <cj...@gmail.com>.
> 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.
Not sure why intelli-j defaults to this behavior but it's fixed.
- Corey
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29502/#review66493
-----------------------------------------------------------
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
>
>