Ok, I'm convinced. I've checked this in. -Eric
On Fri, Jan 25, 2013 at 5:04 PM, John Vines <[email protected]> wrote: > agree with chris > > > On Fri, Jan 25, 2013 at 4:48 PM, Christopher <[email protected]> wrote: > > > +1 for defaulting to the entire table > > > > > > > > -- > > Christopher L Tubbs II > > http://gravatar.com/ctubbsii > > > > > > On Fri, Jan 25, 2013 at 2:30 PM, Eric Newton <[email protected]> > > wrote: > > > > > In the client API, you *must* set ranges on a batch scanner. I wanted > to > > > reflect that requirement in the proxy API. > > > > > > For about an hour yesterday, the range list was in the batch scanner > > > Options. Anyone else want it moved? > > > > > > And, if the range list is empty, we should just use the entire table? > > > > > > -Eric > > > > > > > > > On Fri, Jan 25, 2013 at 2:17 PM, Jason Trost <[email protected]> > > > wrote: > > > > > > > Eric, > > > > > > > > I have started looking at the code, and it looks good. The tests > have > > > good > > > > code coverage. > > > > > > > > Some comments... > > > > > > > > From an API standpoint, why did you split out the Ranges in the > > > > createBatchScanner and not split out the Range in the createScanner. > > > IMO, > > > > these should be consistent. My preference would be for both to use > > their > > > > respective *ScanOptions object to hold the ranges. > > > > > > > > public String createBatchScanner(UserPass userpass, String tableName, > > > > List<org.apache.accumulo.proxy.thrift.Range> pranges, > BatchScanOptions > > > > opts) > > > > > > > > public String createScanner(UserPass userpass, String tableName, > > > > ScanOptions opts) > > > > > > > > Thanks Eric. Other than those comments. I think this looks great and > > > will > > > > be very useful. > > > > > > > > --Jason > > > > > > > > > > > > On Thu, Jan 24, 2013 at 5:36 PM, Eric Newton <[email protected]> > > > > wrote: > > > > > > > > > I've been furiously finishing the Proxy. Please take some time to > > > review > > > > > the Thrift API: making changes to it in the future will be > difficult. > > > > > > > > > > -Eric > > > > > > > > > > > > > > >
