You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by Eric Newton <er...@gmail.com> on 2013/01/24 23:36:00 UTC
Proxy
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
Re: Proxy
Posted by Eric Newton <er...@gmail.com>.
Ok, I'm convinced. I've checked this in.
-Eric
On Fri, Jan 25, 2013 at 5:04 PM, John Vines <vi...@apache.org> wrote:
> agree with chris
>
>
> On Fri, Jan 25, 2013 at 4:48 PM, Christopher <ct...@apache.org> 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 <er...@gmail.com>
> > 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 <ja...@gmail.com>
> > > 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 <er...@gmail.com>
> > > > 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
> > > > >
> > > >
> > >
> >
>
Re: Proxy
Posted by John Vines <vi...@apache.org>.
agree with chris
On Fri, Jan 25, 2013 at 4:48 PM, Christopher <ct...@apache.org> 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 <er...@gmail.com>
> 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 <ja...@gmail.com>
> > 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 <er...@gmail.com>
> > > 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
> > > >
> > >
> >
>
Re: Proxy
Posted by Christopher <ct...@apache.org>.
+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 <er...@gmail.com> 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 <ja...@gmail.com>
> 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 <er...@gmail.com>
> > 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
> > >
> >
>
Re: Proxy
Posted by Eric Newton <er...@gmail.com>.
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 <ja...@gmail.com> 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 <er...@gmail.com>
> 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
> >
>
Re: Proxy
Posted by Jason Trost <ja...@gmail.com>.
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 <er...@gmail.com> 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
>