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