You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by Chris McCubbin <ch...@sqrrl.com> on 2012/11/13 17:37:10 UTC

Review Request: Accumulo proxy source 11/13/12

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

Review request for accumulo.


Description
-------

Patch submitted by Chris McCubbin to add thrift proxy to Accumulo, second version


Diffs
-----

  /trunk/pom.xml 1408812 
  /trunk/proxy/README PRE-CREATION 
  /trunk/proxy/examples/python/README PRE-CREATION 
  /trunk/proxy/examples/python/TestClient.py PRE-CREATION 
  /trunk/proxy/examples/python/data/__init__.py PRE-CREATION 
  /trunk/proxy/examples/python/data/constants.py PRE-CREATION 
  /trunk/proxy/examples/python/data/ttypes.py PRE-CREATION 
  /trunk/proxy/examples/python/proxy/AccumuloProxy-remote PRE-CREATION 
  /trunk/proxy/examples/python/proxy/AccumuloProxy.py PRE-CREATION 
  /trunk/proxy/examples/python/proxy/__init__.py PRE-CREATION 
  /trunk/proxy/examples/python/proxy/constants.py PRE-CREATION 
  /trunk/proxy/examples/python/proxy/ttypes.py PRE-CREATION 
  /trunk/proxy/examples/ruby/README PRE-CREATION 
  /trunk/proxy/examples/ruby/accumulo_proxy.rb PRE-CREATION 
  /trunk/proxy/examples/ruby/data_constants.rb PRE-CREATION 
  /trunk/proxy/examples/ruby/data_types.rb PRE-CREATION 
  /trunk/proxy/examples/ruby/proxy_constants.rb PRE-CREATION 
  /trunk/proxy/examples/ruby/proxy_types.rb PRE-CREATION 
  /trunk/proxy/examples/ruby/test_client.rb PRE-CREATION 
  /trunk/proxy/examples/ruby/thrift.rb PRE-CREATION 
  /trunk/proxy/pom.xml PRE-CREATION 
  /trunk/proxy/proxy.properties PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyHarness.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/TestProxyClient.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/Util.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloException.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloProxy.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloSecurityException.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/IOException.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/KeyValueAndPeek.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/NoMoreEntriesException.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKey.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKeyValue.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PMutation.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PRange.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PScanResult.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/ProxyIteratorSetting.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/ProxySystemPermission.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/ProxyTablePermission.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableExistsException.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableNotFoundException.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/UserPass.java PRE-CREATION 
  /trunk/proxy/src/main/thrift/proxy.thrift PRE-CREATION 
  /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyInstanceOperations.java PRE-CREATION 
  /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyReadWrite.java PRE-CREATION 
  /trunk/proxy/src/test/java/org/apache/accumulo/TestProxySecurityOperations.java PRE-CREATION 
  /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyTableOperations.java PRE-CREATION 

Diff: https://reviews.apache.org/r/8039/diff/


Testing
-------


Thanks,

Chris McCubbin


Re: Review Request: Accumulo proxy source 11/13/12

Posted by Chris McCubbin <ch...@sqrrl.com>.

> On Nov. 13, 2012, 8:57 p.m., kturner wrote:
> > /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java, line 516
> > <https://reviews.apache.org/r/8039/diff/1/?file=189099#file189099line516>
> >
> >     seems confusing to accept a range of keys and then only use the row portion of the range...  eithier use the entire key or compose PRange out of rows...

Yeah, I had actually intended to use the whole key. not sure what happened there. Changed this just now.


- Chris


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


On Nov. 13, 2012, 4:37 p.m., Chris McCubbin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8039/
> -----------------------------------------------------------
> 
> (Updated Nov. 13, 2012, 4:37 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Description
> -------
> 
> Patch submitted by Chris McCubbin to add thrift proxy to Accumulo, second version
> 
> 
> Diffs
> -----
> 
>   /trunk/pom.xml 1408812 
>   /trunk/proxy/README PRE-CREATION 
>   /trunk/proxy/examples/python/README PRE-CREATION 
>   /trunk/proxy/examples/python/TestClient.py PRE-CREATION 
>   /trunk/proxy/examples/python/data/__init__.py PRE-CREATION 
>   /trunk/proxy/examples/python/data/constants.py PRE-CREATION 
>   /trunk/proxy/examples/python/data/ttypes.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/AccumuloProxy-remote PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/AccumuloProxy.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/__init__.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/constants.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/ttypes.py PRE-CREATION 
>   /trunk/proxy/examples/ruby/README PRE-CREATION 
>   /trunk/proxy/examples/ruby/accumulo_proxy.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/data_constants.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/data_types.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/proxy_constants.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/proxy_types.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/test_client.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/thrift.rb PRE-CREATION 
>   /trunk/proxy/pom.xml PRE-CREATION 
>   /trunk/proxy/proxy.properties PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyHarness.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/TestProxyClient.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/Util.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloProxy.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloSecurityException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/IOException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/KeyValueAndPeek.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/NoMoreEntriesException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKey.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKeyValue.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PMutation.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PRange.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PScanResult.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/ProxyIteratorSetting.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/ProxySystemPermission.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/ProxyTablePermission.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableExistsException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableNotFoundException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/UserPass.java PRE-CREATION 
>   /trunk/proxy/src/main/thrift/proxy.thrift PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyInstanceOperations.java PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyReadWrite.java PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxySecurityOperations.java PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyTableOperations.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/8039/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chris McCubbin
> 
>


Re: Review Request: Accumulo proxy source 11/13/12

Posted by Chris McCubbin <ch...@sqrrl.com>.

> On Nov. 13, 2012, 8:57 p.m., kturner wrote:
> > /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java, line 575
> > <https://reviews.apache.org/r/8039/diff/1/?file=189099#file189099line575>
> >
> >     numRead does not seem to be incremented...
> >     
> >     If numRead were incremented, I think its possible that (numRead == k) could be true when there is no more data... maybe do ret.setMore(batchScanner.hasNext())

This is a bug I found as well. I fixed it and added a unit test check for this issue.


- Chris


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


On Nov. 13, 2012, 4:37 p.m., Chris McCubbin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8039/
> -----------------------------------------------------------
> 
> (Updated Nov. 13, 2012, 4:37 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Description
> -------
> 
> Patch submitted by Chris McCubbin to add thrift proxy to Accumulo, second version
> 
> 
> Diffs
> -----
> 
>   /trunk/pom.xml 1408812 
>   /trunk/proxy/README PRE-CREATION 
>   /trunk/proxy/examples/python/README PRE-CREATION 
>   /trunk/proxy/examples/python/TestClient.py PRE-CREATION 
>   /trunk/proxy/examples/python/data/__init__.py PRE-CREATION 
>   /trunk/proxy/examples/python/data/constants.py PRE-CREATION 
>   /trunk/proxy/examples/python/data/ttypes.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/AccumuloProxy-remote PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/AccumuloProxy.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/__init__.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/constants.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/ttypes.py PRE-CREATION 
>   /trunk/proxy/examples/ruby/README PRE-CREATION 
>   /trunk/proxy/examples/ruby/accumulo_proxy.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/data_constants.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/data_types.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/proxy_constants.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/proxy_types.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/test_client.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/thrift.rb PRE-CREATION 
>   /trunk/proxy/pom.xml PRE-CREATION 
>   /trunk/proxy/proxy.properties PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyHarness.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/TestProxyClient.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/Util.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloProxy.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloSecurityException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/IOException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/KeyValueAndPeek.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/NoMoreEntriesException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKey.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKeyValue.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PMutation.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PRange.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PScanResult.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/ProxyIteratorSetting.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/ProxySystemPermission.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/ProxyTablePermission.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableExistsException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableNotFoundException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/UserPass.java PRE-CREATION 
>   /trunk/proxy/src/main/thrift/proxy.thrift PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyInstanceOperations.java PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyReadWrite.java PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxySecurityOperations.java PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyTableOperations.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/8039/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chris McCubbin
> 
>


Re: Review Request: Accumulo proxy source 11/13/12

Posted by Chris McCubbin <ch...@sqrrl.com>.

> On Nov. 13, 2012, 8:57 p.m., kturner wrote:
> > /trunk/proxy/src/main/thrift/proxy.thrift, line 16
> > <https://reviews.apache.org/r/8039/diff/1/?file=189119#file189119line16>
> >
> >     How does the user indicate that they would like the system to set the timestamp?

timestamp is now an optional field.


- Chris


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


On Nov. 13, 2012, 4:37 p.m., Chris McCubbin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8039/
> -----------------------------------------------------------
> 
> (Updated Nov. 13, 2012, 4:37 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Description
> -------
> 
> Patch submitted by Chris McCubbin to add thrift proxy to Accumulo, second version
> 
> 
> Diffs
> -----
> 
>   /trunk/pom.xml 1408812 
>   /trunk/proxy/README PRE-CREATION 
>   /trunk/proxy/examples/python/README PRE-CREATION 
>   /trunk/proxy/examples/python/TestClient.py PRE-CREATION 
>   /trunk/proxy/examples/python/data/__init__.py PRE-CREATION 
>   /trunk/proxy/examples/python/data/constants.py PRE-CREATION 
>   /trunk/proxy/examples/python/data/ttypes.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/AccumuloProxy-remote PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/AccumuloProxy.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/__init__.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/constants.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/ttypes.py PRE-CREATION 
>   /trunk/proxy/examples/ruby/README PRE-CREATION 
>   /trunk/proxy/examples/ruby/accumulo_proxy.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/data_constants.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/data_types.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/proxy_constants.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/proxy_types.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/test_client.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/thrift.rb PRE-CREATION 
>   /trunk/proxy/pom.xml PRE-CREATION 
>   /trunk/proxy/proxy.properties PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyHarness.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/TestProxyClient.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/Util.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloProxy.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloSecurityException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/IOException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/KeyValueAndPeek.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/NoMoreEntriesException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKey.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKeyValue.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PMutation.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PRange.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PScanResult.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/ProxyIteratorSetting.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/ProxySystemPermission.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/ProxyTablePermission.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableExistsException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableNotFoundException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/UserPass.java PRE-CREATION 
>   /trunk/proxy/src/main/thrift/proxy.thrift PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyInstanceOperations.java PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyReadWrite.java PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxySecurityOperations.java PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyTableOperations.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/8039/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chris McCubbin
> 
>


Re: Review Request: Accumulo proxy source 11/13/12

Posted by ke...@deenlo.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8039/#review13399
-----------------------------------------------------------



/trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java
<https://reviews.apache.org/r/8039/#comment28732>

    seems confusing to accept a range of keys and then only use the row portion of the range...  eithier use the entire key or compose PRange out of rows...



/trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java
<https://reviews.apache.org/r/8039/#comment28733>

    numRead does not seem to be incremented...
    
    If numRead were incremented, I think its possible that (numRead == k) could be true when there is no more data... maybe do ret.setMore(batchScanner.hasNext())



/trunk/proxy/src/main/thrift/proxy.thrift
<https://reviews.apache.org/r/8039/#comment28728>

    How does the user indicate that they would like the system to set the timestamp?



/trunk/proxy/src/main/thrift/proxy.thrift
<https://reviews.apache.org/r/8039/#comment28730>

    I would advocate adding the following and making mutation use these 
    
    struct PCoumnUpdate {
       1:binary cf,
       2: binary cq,
       3:binary cv,
       4:i64 ts
       5:bool useSystemTime
       6:binary value
    }
    
    struct PColumnDelete {
       1:binary cf,
       2: binary cq,
       3:binary cv,
       4:i64 ts
       5:bool useSystemTime
    }



/trunk/proxy/src/main/thrift/proxy.thrift
<https://reviews.apache.org/r/8039/#comment28731>

    need a method to create a normal scanner in addition to the method to create a batch scanner



/trunk/proxy/src/main/thrift/proxy.thrift
<https://reviews.apache.org/r/8039/#comment28729>

    should take a list of ranges


- kturner


On Nov. 13, 2012, 4:37 p.m., Chris McCubbin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8039/
> -----------------------------------------------------------
> 
> (Updated Nov. 13, 2012, 4:37 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Description
> -------
> 
> Patch submitted by Chris McCubbin to add thrift proxy to Accumulo, second version
> 
> 
> Diffs
> -----
> 
>   /trunk/pom.xml 1408812 
>   /trunk/proxy/README PRE-CREATION 
>   /trunk/proxy/examples/python/README PRE-CREATION 
>   /trunk/proxy/examples/python/TestClient.py PRE-CREATION 
>   /trunk/proxy/examples/python/data/__init__.py PRE-CREATION 
>   /trunk/proxy/examples/python/data/constants.py PRE-CREATION 
>   /trunk/proxy/examples/python/data/ttypes.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/AccumuloProxy-remote PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/AccumuloProxy.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/__init__.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/constants.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/ttypes.py PRE-CREATION 
>   /trunk/proxy/examples/ruby/README PRE-CREATION 
>   /trunk/proxy/examples/ruby/accumulo_proxy.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/data_constants.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/data_types.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/proxy_constants.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/proxy_types.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/test_client.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/thrift.rb PRE-CREATION 
>   /trunk/proxy/pom.xml PRE-CREATION 
>   /trunk/proxy/proxy.properties PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyHarness.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/TestProxyClient.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/Util.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloProxy.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloSecurityException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/IOException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/KeyValueAndPeek.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/NoMoreEntriesException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKey.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKeyValue.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PMutation.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PRange.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PScanResult.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/ProxyIteratorSetting.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/ProxySystemPermission.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/ProxyTablePermission.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableExistsException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableNotFoundException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/UserPass.java PRE-CREATION 
>   /trunk/proxy/src/main/thrift/proxy.thrift PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyInstanceOperations.java PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyReadWrite.java PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxySecurityOperations.java PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyTableOperations.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/8039/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chris McCubbin
> 
>


Re: Review Request: Accumulo proxy source 11/13/12

Posted by Chris McCubbin <ch...@sqrrl.com>.

> On Nov. 14, 2012, 7:28 p.m., kturner wrote:
> > /trunk/proxy/src/main/thrift/proxy.thrift, line 21
> > <https://reviews.apache.org/r/8039/diff/2/?file=189891#file189891line21>
> >
> >     Creating ranges can be tricky.  It would be nice to provide thrift helper methods for creating ranges.  For example could have a  methods like the following.
> >     
> >       PRange rowRange(binary startRow, binary endRow)
> >       PRange prefixRange(binary row);
> >     
> >     You can look at the helper functions in Range.  There are a lot of them.  At least want to provide ones for rows.
> >     
> >     Could possibly provide the followingKey and followingPrefix functions that most helper functions in Range build on.
> >     
> >     Also, why not have inclusive/exclusive booleans for range?
> 
> Adam Fuchs wrote:
>     The convenience constructors for range are very helpful, and they should definitely be included in the proxy code.
>     
>     Inclusive booleans are unnecessary since keys have well-defined successors (even if they don't have predecessors). Having the start be inclusive and the end be exclusive is sufficient, and simplifies the API. We should, however, have successor accessible as a method on PKey.
> 
> kturner wrote:
>     Need to document that range is inclusive/exclusive.   If user had something like followingKey(), then they could effectively create an exclusive start or inclusive end.

Structs in thrift are not objects and do not have methods on them, nor can they inherit from each other. Convenience methods like the ones you guys are describing would have to be implemented either in the proxy server and exposed via the API or re-implemented for every language.


- Chris


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


On Nov. 14, 2012, 6:51 p.m., Chris McCubbin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8039/
> -----------------------------------------------------------
> 
> (Updated Nov. 14, 2012, 6:51 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Description
> -------
> 
> Patch submitted by Chris McCubbin to add thrift proxy to Accumulo, second version
> 
> 
> Diffs
> -----
> 
>   /trunk/pom.xml 1409290 
>   /trunk/proxy/README PRE-CREATION 
>   /trunk/proxy/accumulo-proxy.iml PRE-CREATION 
>   /trunk/proxy/examples/python/README PRE-CREATION 
>   /trunk/proxy/examples/python/TestClient.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/AccumuloProxy-remote PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/AccumuloProxy.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/__init__.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/constants.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/ttypes.py PRE-CREATION 
>   /trunk/proxy/examples/ruby/README PRE-CREATION 
>   /trunk/proxy/examples/ruby/accumulo_proxy.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/proxy_constants.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/proxy_types.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/test_client.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/thrift.rb PRE-CREATION 
>   /trunk/proxy/pom.xml PRE-CREATION 
>   /trunk/proxy/proxy.properties PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyHarness.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/TestProxyClient.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/Util.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloProxy.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloSecurityException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/IOException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/KeyValueAndPeek.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/NoMoreEntriesException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PIteratorSetting.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKey.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKeyValue.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PRange.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PScanResult.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PSystemPermission.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PTablePermission.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableExistsException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableNotFoundException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/UserPass.java PRE-CREATION 
>   /trunk/proxy/src/main/thrift/proxy.thrift PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyInstanceOperations.java PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyReadWrite.java PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxySecurityOperations.java PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyTableOperations.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/8039/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chris McCubbin
> 
>


Re: Review Request: Accumulo proxy source 11/13/12

Posted by Keith Turner <ke...@deenlo.com>.
On Thu, Nov 15, 2012 at 12:40 PM, Billie Rinaldi <bi...@apache.org> wrote:
> On Wed, Nov 14, 2012 at 12:34 PM, <ke...@deenlo.com> wrote:
>
>>
>>
>> > On Nov. 14, 2012, 7:28 p.m., kturner wrote:
>> > > /trunk/proxy/src/main/thrift/proxy.thrift, line 21
>> > > <
>> https://reviews.apache.org/r/8039/diff/2/?file=189891#file189891line21>
>> > >
>> > >     Creating ranges can be tricky.  It would be nice to provide thrift
>> helper methods for creating ranges.  For example could have a  methods like
>> the following.
>> > >
>> > >       PRange rowRange(binary startRow, binary endRow)
>> > >       PRange prefixRange(binary row);
>> > >
>> > >     You can look at the helper functions in Range.  There are a lot of
>> them.  At least want to provide ones for rows.
>> > >
>> > >     Could possibly provide the followingKey and followingPrefix
>> functions that most helper functions in Range build on.
>> > >
>> > >     Also, why not have inclusive/exclusive booleans for range?
>> >
>> > Adam Fuchs wrote:
>> >     The convenience constructors for range are very helpful, and they
>> should definitely be included in the proxy code.
>> >
>> >     Inclusive booleans are unnecessary since keys have well-defined
>> successors (even if they don't have predecessors). Having the start be
>> inclusive and the end be exclusive is sufficient, and simplifies the API.
>> We should, however, have successor accessible as a method on PKey.
>> >
>> > kturner wrote:
>> >     Need to document that range is inclusive/exclusive.   If user had
>> something like followingKey(), then they could effectively create an
>> exclusive start or inclusive end.
>> >
>> > Chris McCubbin wrote:
>> >     Structs in thrift are not objects and do not have methods on them,
>> nor can they inherit from each other. Convenience methods like the ones you
>> guys are describing would have to be implemented either in the proxy server
>> and exposed via the API or re-implemented for every language.
>>
>> I was thinking these would just be more methods on the proxy service.
>>
>
> Using a successor / followingKey method instead of an inclusive boolean
> does require the user to understand PartialKey and to use it
> appropriately.  But maybe that's OK.

In addition to documenting that PRange is inclusive/exclusive in
thrift file.  Should also include some documentation or pointers in
thrift file about how a succesor can effectively change inclusive to
exclusive and visa/versa.  New users would probably not be aware of
this possibility.

>
> Billie
>
>
>
>>
>>
>> - kturner
>>
>>
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/8039/#review13433
>> -----------------------------------------------------------
>>
>>
>> On Nov. 14, 2012, 6:51 p.m., Chris McCubbin wrote:
>> >
>> > -----------------------------------------------------------
>> > This is an automatically generated e-mail. To reply, visit:
>> > https://reviews.apache.org/r/8039/
>> > -----------------------------------------------------------
>> >
>> > (Updated Nov. 14, 2012, 6:51 p.m.)
>> >
>> >
>> > Review request for accumulo.
>> >
>> >
>> > Description
>> > -------
>> >
>> > Patch submitted by Chris McCubbin to add thrift proxy to Accumulo,
>> second version
>> >
>> >
>> > Diffs
>> > -----
>> >
>> >   /trunk/pom.xml 1409290
>> >   /trunk/proxy/README PRE-CREATION
>> >   /trunk/proxy/accumulo-proxy.iml PRE-CREATION
>> >   /trunk/proxy/examples/python/README PRE-CREATION
>> >   /trunk/proxy/examples/python/TestClient.py PRE-CREATION
>> >   /trunk/proxy/examples/python/proxy/AccumuloProxy-remote PRE-CREATION
>> >   /trunk/proxy/examples/python/proxy/AccumuloProxy.py PRE-CREATION
>> >   /trunk/proxy/examples/python/proxy/__init__.py PRE-CREATION
>> >   /trunk/proxy/examples/python/proxy/constants.py PRE-CREATION
>> >   /trunk/proxy/examples/python/proxy/ttypes.py PRE-CREATION
>> >   /trunk/proxy/examples/ruby/README PRE-CREATION
>> >   /trunk/proxy/examples/ruby/accumulo_proxy.rb PRE-CREATION
>> >   /trunk/proxy/examples/ruby/proxy_constants.rb PRE-CREATION
>> >   /trunk/proxy/examples/ruby/proxy_types.rb PRE-CREATION
>> >   /trunk/proxy/examples/ruby/test_client.rb PRE-CREATION
>> >   /trunk/proxy/examples/ruby/thrift.rb PRE-CREATION
>> >   /trunk/proxy/pom.xml PRE-CREATION
>> >   /trunk/proxy/proxy.properties PRE-CREATION
>> >   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyHarness.java
>> PRE-CREATION
>> >   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java
>> PRE-CREATION
>> >
>> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/TestProxyClient.java
>> PRE-CREATION
>> >   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/Util.java
>> PRE-CREATION
>> >
>> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloException.java
>> PRE-CREATION
>> >
>> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloProxy.java
>> PRE-CREATION
>> >
>> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloSecurityException.java
>> PRE-CREATION
>> >
>> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/IOException.java
>> PRE-CREATION
>> >
>> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/KeyValueAndPeek.java
>> PRE-CREATION
>> >
>> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/NoMoreEntriesException.java
>> PRE-CREATION
>> >
>> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PIteratorSetting.java
>> PRE-CREATION
>> >   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKey.java
>> PRE-CREATION
>> >
>> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKeyValue.java
>> PRE-CREATION
>> >   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PRange.java
>> PRE-CREATION
>> >
>> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PScanResult.java
>> PRE-CREATION
>> >
>> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PSystemPermission.java
>> PRE-CREATION
>> >
>> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PTablePermission.java
>> PRE-CREATION
>> >
>> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableExistsException.java
>> PRE-CREATION
>> >
>> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableNotFoundException.java
>> PRE-CREATION
>> >   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/UserPass.java
>> PRE-CREATION
>> >   /trunk/proxy/src/main/thrift/proxy.thrift PRE-CREATION
>> >
>> /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyInstanceOperations.java
>> PRE-CREATION
>> >   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyReadWrite.java
>> PRE-CREATION
>> >
>> /trunk/proxy/src/test/java/org/apache/accumulo/TestProxySecurityOperations.java
>> PRE-CREATION
>> >
>> /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyTableOperations.java
>> PRE-CREATION
>> >
>> > Diff: https://reviews.apache.org/r/8039/diff/
>> >
>> >
>> > Testing
>> > -------
>> >
>> >
>> > Thanks,
>> >
>> > Chris McCubbin
>> >
>> >
>>
>>

Re: Review Request: Accumulo proxy source 11/13/12

Posted by Billie Rinaldi <bi...@apache.org>.
On Wed, Nov 14, 2012 at 12:34 PM, <ke...@deenlo.com> wrote:

>
>
> > On Nov. 14, 2012, 7:28 p.m., kturner wrote:
> > > /trunk/proxy/src/main/thrift/proxy.thrift, line 21
> > > <
> https://reviews.apache.org/r/8039/diff/2/?file=189891#file189891line21>
> > >
> > >     Creating ranges can be tricky.  It would be nice to provide thrift
> helper methods for creating ranges.  For example could have a  methods like
> the following.
> > >
> > >       PRange rowRange(binary startRow, binary endRow)
> > >       PRange prefixRange(binary row);
> > >
> > >     You can look at the helper functions in Range.  There are a lot of
> them.  At least want to provide ones for rows.
> > >
> > >     Could possibly provide the followingKey and followingPrefix
> functions that most helper functions in Range build on.
> > >
> > >     Also, why not have inclusive/exclusive booleans for range?
> >
> > Adam Fuchs wrote:
> >     The convenience constructors for range are very helpful, and they
> should definitely be included in the proxy code.
> >
> >     Inclusive booleans are unnecessary since keys have well-defined
> successors (even if they don't have predecessors). Having the start be
> inclusive and the end be exclusive is sufficient, and simplifies the API.
> We should, however, have successor accessible as a method on PKey.
> >
> > kturner wrote:
> >     Need to document that range is inclusive/exclusive.   If user had
> something like followingKey(), then they could effectively create an
> exclusive start or inclusive end.
> >
> > Chris McCubbin wrote:
> >     Structs in thrift are not objects and do not have methods on them,
> nor can they inherit from each other. Convenience methods like the ones you
> guys are describing would have to be implemented either in the proxy server
> and exposed via the API or re-implemented for every language.
>
> I was thinking these would just be more methods on the proxy service.
>

Using a successor / followingKey method instead of an inclusive boolean
does require the user to understand PartialKey and to use it
appropriately.  But maybe that's OK.

Billie



>
>
> - kturner
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8039/#review13433
> -----------------------------------------------------------
>
>
> On Nov. 14, 2012, 6:51 p.m., Chris McCubbin wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/8039/
> > -----------------------------------------------------------
> >
> > (Updated Nov. 14, 2012, 6:51 p.m.)
> >
> >
> > Review request for accumulo.
> >
> >
> > Description
> > -------
> >
> > Patch submitted by Chris McCubbin to add thrift proxy to Accumulo,
> second version
> >
> >
> > Diffs
> > -----
> >
> >   /trunk/pom.xml 1409290
> >   /trunk/proxy/README PRE-CREATION
> >   /trunk/proxy/accumulo-proxy.iml PRE-CREATION
> >   /trunk/proxy/examples/python/README PRE-CREATION
> >   /trunk/proxy/examples/python/TestClient.py PRE-CREATION
> >   /trunk/proxy/examples/python/proxy/AccumuloProxy-remote PRE-CREATION
> >   /trunk/proxy/examples/python/proxy/AccumuloProxy.py PRE-CREATION
> >   /trunk/proxy/examples/python/proxy/__init__.py PRE-CREATION
> >   /trunk/proxy/examples/python/proxy/constants.py PRE-CREATION
> >   /trunk/proxy/examples/python/proxy/ttypes.py PRE-CREATION
> >   /trunk/proxy/examples/ruby/README PRE-CREATION
> >   /trunk/proxy/examples/ruby/accumulo_proxy.rb PRE-CREATION
> >   /trunk/proxy/examples/ruby/proxy_constants.rb PRE-CREATION
> >   /trunk/proxy/examples/ruby/proxy_types.rb PRE-CREATION
> >   /trunk/proxy/examples/ruby/test_client.rb PRE-CREATION
> >   /trunk/proxy/examples/ruby/thrift.rb PRE-CREATION
> >   /trunk/proxy/pom.xml PRE-CREATION
> >   /trunk/proxy/proxy.properties PRE-CREATION
> >   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyHarness.java
> PRE-CREATION
> >   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java
> PRE-CREATION
> >
> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/TestProxyClient.java
> PRE-CREATION
> >   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/Util.java
> PRE-CREATION
> >
> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloException.java
> PRE-CREATION
> >
> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloProxy.java
> PRE-CREATION
> >
> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloSecurityException.java
> PRE-CREATION
> >
> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/IOException.java
> PRE-CREATION
> >
> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/KeyValueAndPeek.java
> PRE-CREATION
> >
> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/NoMoreEntriesException.java
> PRE-CREATION
> >
> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PIteratorSetting.java
> PRE-CREATION
> >   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKey.java
> PRE-CREATION
> >
> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKeyValue.java
> PRE-CREATION
> >   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PRange.java
> PRE-CREATION
> >
> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PScanResult.java
> PRE-CREATION
> >
> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PSystemPermission.java
> PRE-CREATION
> >
> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PTablePermission.java
> PRE-CREATION
> >
> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableExistsException.java
> PRE-CREATION
> >
> /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableNotFoundException.java
> PRE-CREATION
> >   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/UserPass.java
> PRE-CREATION
> >   /trunk/proxy/src/main/thrift/proxy.thrift PRE-CREATION
> >
> /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyInstanceOperations.java
> PRE-CREATION
> >   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyReadWrite.java
> PRE-CREATION
> >
> /trunk/proxy/src/test/java/org/apache/accumulo/TestProxySecurityOperations.java
> PRE-CREATION
> >
> /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyTableOperations.java
> PRE-CREATION
> >
> > Diff: https://reviews.apache.org/r/8039/diff/
> >
> >
> > Testing
> > -------
> >
> >
> > Thanks,
> >
> > Chris McCubbin
> >
> >
>
>

Re: Review Request: Accumulo proxy source 11/13/12

Posted by ke...@deenlo.com.

> On Nov. 14, 2012, 7:28 p.m., kturner wrote:
> > /trunk/proxy/src/main/thrift/proxy.thrift, line 21
> > <https://reviews.apache.org/r/8039/diff/2/?file=189891#file189891line21>
> >
> >     Creating ranges can be tricky.  It would be nice to provide thrift helper methods for creating ranges.  For example could have a  methods like the following.
> >     
> >       PRange rowRange(binary startRow, binary endRow)
> >       PRange prefixRange(binary row);
> >     
> >     You can look at the helper functions in Range.  There are a lot of them.  At least want to provide ones for rows.
> >     
> >     Could possibly provide the followingKey and followingPrefix functions that most helper functions in Range build on.
> >     
> >     Also, why not have inclusive/exclusive booleans for range?
> 
> Adam Fuchs wrote:
>     The convenience constructors for range are very helpful, and they should definitely be included in the proxy code.
>     
>     Inclusive booleans are unnecessary since keys have well-defined successors (even if they don't have predecessors). Having the start be inclusive and the end be exclusive is sufficient, and simplifies the API. We should, however, have successor accessible as a method on PKey.
> 
> kturner wrote:
>     Need to document that range is inclusive/exclusive.   If user had something like followingKey(), then they could effectively create an exclusive start or inclusive end.
> 
> Chris McCubbin wrote:
>     Structs in thrift are not objects and do not have methods on them, nor can they inherit from each other. Convenience methods like the ones you guys are describing would have to be implemented either in the proxy server and exposed via the API or re-implemented for every language.

I was thinking these would just be more methods on the proxy service.


- kturner


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


On Nov. 14, 2012, 6:51 p.m., Chris McCubbin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8039/
> -----------------------------------------------------------
> 
> (Updated Nov. 14, 2012, 6:51 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Description
> -------
> 
> Patch submitted by Chris McCubbin to add thrift proxy to Accumulo, second version
> 
> 
> Diffs
> -----
> 
>   /trunk/pom.xml 1409290 
>   /trunk/proxy/README PRE-CREATION 
>   /trunk/proxy/accumulo-proxy.iml PRE-CREATION 
>   /trunk/proxy/examples/python/README PRE-CREATION 
>   /trunk/proxy/examples/python/TestClient.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/AccumuloProxy-remote PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/AccumuloProxy.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/__init__.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/constants.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/ttypes.py PRE-CREATION 
>   /trunk/proxy/examples/ruby/README PRE-CREATION 
>   /trunk/proxy/examples/ruby/accumulo_proxy.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/proxy_constants.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/proxy_types.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/test_client.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/thrift.rb PRE-CREATION 
>   /trunk/proxy/pom.xml PRE-CREATION 
>   /trunk/proxy/proxy.properties PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyHarness.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/TestProxyClient.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/Util.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloProxy.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloSecurityException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/IOException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/KeyValueAndPeek.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/NoMoreEntriesException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PIteratorSetting.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKey.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKeyValue.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PRange.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PScanResult.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PSystemPermission.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PTablePermission.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableExistsException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableNotFoundException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/UserPass.java PRE-CREATION 
>   /trunk/proxy/src/main/thrift/proxy.thrift PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyInstanceOperations.java PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyReadWrite.java PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxySecurityOperations.java PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyTableOperations.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/8039/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chris McCubbin
> 
>


Re: Review Request: Accumulo proxy source 11/13/12

Posted by Adam Fuchs <af...@apache.org>.

> On Nov. 14, 2012, 7:28 p.m., kturner wrote:
> > /trunk/proxy/src/main/thrift/proxy.thrift, line 21
> > <https://reviews.apache.org/r/8039/diff/2/?file=189891#file189891line21>
> >
> >     Creating ranges can be tricky.  It would be nice to provide thrift helper methods for creating ranges.  For example could have a  methods like the following.
> >     
> >       PRange rowRange(binary startRow, binary endRow)
> >       PRange prefixRange(binary row);
> >     
> >     You can look at the helper functions in Range.  There are a lot of them.  At least want to provide ones for rows.
> >     
> >     Could possibly provide the followingKey and followingPrefix functions that most helper functions in Range build on.
> >     
> >     Also, why not have inclusive/exclusive booleans for range?

The convenience constructors for range are very helpful, and they should definitely be included in the proxy code.

Inclusive booleans are unnecessary since keys have well-defined successors (even if they don't have predecessors). Having the start be inclusive and the end be exclusive is sufficient, and simplifies the API. We should, however, have successor accessible as a method on PKey.


- Adam


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


On Nov. 14, 2012, 6:51 p.m., Chris McCubbin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8039/
> -----------------------------------------------------------
> 
> (Updated Nov. 14, 2012, 6:51 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Description
> -------
> 
> Patch submitted by Chris McCubbin to add thrift proxy to Accumulo, second version
> 
> 
> Diffs
> -----
> 
>   /trunk/pom.xml 1409290 
>   /trunk/proxy/README PRE-CREATION 
>   /trunk/proxy/accumulo-proxy.iml PRE-CREATION 
>   /trunk/proxy/examples/python/README PRE-CREATION 
>   /trunk/proxy/examples/python/TestClient.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/AccumuloProxy-remote PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/AccumuloProxy.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/__init__.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/constants.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/ttypes.py PRE-CREATION 
>   /trunk/proxy/examples/ruby/README PRE-CREATION 
>   /trunk/proxy/examples/ruby/accumulo_proxy.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/proxy_constants.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/proxy_types.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/test_client.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/thrift.rb PRE-CREATION 
>   /trunk/proxy/pom.xml PRE-CREATION 
>   /trunk/proxy/proxy.properties PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyHarness.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/TestProxyClient.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/Util.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloProxy.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloSecurityException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/IOException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/KeyValueAndPeek.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/NoMoreEntriesException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PIteratorSetting.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKey.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKeyValue.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PRange.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PScanResult.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PSystemPermission.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PTablePermission.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableExistsException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableNotFoundException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/UserPass.java PRE-CREATION 
>   /trunk/proxy/src/main/thrift/proxy.thrift PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyInstanceOperations.java PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyReadWrite.java PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxySecurityOperations.java PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyTableOperations.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/8039/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chris McCubbin
> 
>


Re: Review Request: Accumulo proxy source 11/13/12

Posted by ke...@deenlo.com.

> On Nov. 14, 2012, 7:28 p.m., kturner wrote:
> > /trunk/proxy/src/main/thrift/proxy.thrift, line 21
> > <https://reviews.apache.org/r/8039/diff/2/?file=189891#file189891line21>
> >
> >     Creating ranges can be tricky.  It would be nice to provide thrift helper methods for creating ranges.  For example could have a  methods like the following.
> >     
> >       PRange rowRange(binary startRow, binary endRow)
> >       PRange prefixRange(binary row);
> >     
> >     You can look at the helper functions in Range.  There are a lot of them.  At least want to provide ones for rows.
> >     
> >     Could possibly provide the followingKey and followingPrefix functions that most helper functions in Range build on.
> >     
> >     Also, why not have inclusive/exclusive booleans for range?
> 
> Adam Fuchs wrote:
>     The convenience constructors for range are very helpful, and they should definitely be included in the proxy code.
>     
>     Inclusive booleans are unnecessary since keys have well-defined successors (even if they don't have predecessors). Having the start be inclusive and the end be exclusive is sufficient, and simplifies the API. We should, however, have successor accessible as a method on PKey.

Need to document that range is inclusive/exclusive.   If user had something like followingKey(), then they could effectively create an exclusive start or inclusive end.


- kturner


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


On Nov. 14, 2012, 6:51 p.m., Chris McCubbin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8039/
> -----------------------------------------------------------
> 
> (Updated Nov. 14, 2012, 6:51 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Description
> -------
> 
> Patch submitted by Chris McCubbin to add thrift proxy to Accumulo, second version
> 
> 
> Diffs
> -----
> 
>   /trunk/pom.xml 1409290 
>   /trunk/proxy/README PRE-CREATION 
>   /trunk/proxy/accumulo-proxy.iml PRE-CREATION 
>   /trunk/proxy/examples/python/README PRE-CREATION 
>   /trunk/proxy/examples/python/TestClient.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/AccumuloProxy-remote PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/AccumuloProxy.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/__init__.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/constants.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/ttypes.py PRE-CREATION 
>   /trunk/proxy/examples/ruby/README PRE-CREATION 
>   /trunk/proxy/examples/ruby/accumulo_proxy.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/proxy_constants.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/proxy_types.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/test_client.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/thrift.rb PRE-CREATION 
>   /trunk/proxy/pom.xml PRE-CREATION 
>   /trunk/proxy/proxy.properties PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyHarness.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/TestProxyClient.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/Util.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloProxy.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloSecurityException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/IOException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/KeyValueAndPeek.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/NoMoreEntriesException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PIteratorSetting.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKey.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKeyValue.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PRange.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PScanResult.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PSystemPermission.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PTablePermission.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableExistsException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableNotFoundException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/UserPass.java PRE-CREATION 
>   /trunk/proxy/src/main/thrift/proxy.thrift PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyInstanceOperations.java PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyReadWrite.java PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxySecurityOperations.java PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyTableOperations.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/8039/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chris McCubbin
> 
>


Re: Review Request: Accumulo proxy source 11/13/12

Posted by ke...@deenlo.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8039/#review13433
-----------------------------------------------------------



/trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java
<https://reviews.apache.org/r/8039/#comment28788>

    Could  do a get here instead of contains key and put if null. Remember the result of the get and user later.  Also remember the mutation when doing put.  This way just one lookup in hashmap is done.   



/trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java
<https://reviews.apache.org/r/8039/#comment28789>

    Might be worthwhile to have a map of ColumnVis and resuse them.  Can be expensive to construct ColumnVis because it verifies expression.   Lookup in a hash map is cheaper.   Colvis Hashmap could be local to function.



/trunk/proxy/src/main/thrift/proxy.thrift
<https://reviews.apache.org/r/8039/#comment28787>

    Creating ranges can be tricky.  It would be nice to provide thrift helper methods for creating ranges.  For example could have a  methods like the following.
    
      PRange rowRange(binary startRow, binary endRow)
      PRange prefixRange(binary row);
    
    You can look at the helper functions in Range.  There are a lot of them.  At least want to provide ones for rows.
    
    Could possibly provide the followingKey and followingPrefix functions that most helper functions in Range build on.
    
    Also, why not have inclusive/exclusive booleans for range?


- kturner


On Nov. 14, 2012, 6:51 p.m., Chris McCubbin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8039/
> -----------------------------------------------------------
> 
> (Updated Nov. 14, 2012, 6:51 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Description
> -------
> 
> Patch submitted by Chris McCubbin to add thrift proxy to Accumulo, second version
> 
> 
> Diffs
> -----
> 
>   /trunk/pom.xml 1409290 
>   /trunk/proxy/README PRE-CREATION 
>   /trunk/proxy/accumulo-proxy.iml PRE-CREATION 
>   /trunk/proxy/examples/python/README PRE-CREATION 
>   /trunk/proxy/examples/python/TestClient.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/AccumuloProxy-remote PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/AccumuloProxy.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/__init__.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/constants.py PRE-CREATION 
>   /trunk/proxy/examples/python/proxy/ttypes.py PRE-CREATION 
>   /trunk/proxy/examples/ruby/README PRE-CREATION 
>   /trunk/proxy/examples/ruby/accumulo_proxy.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/proxy_constants.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/proxy_types.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/test_client.rb PRE-CREATION 
>   /trunk/proxy/examples/ruby/thrift.rb PRE-CREATION 
>   /trunk/proxy/pom.xml PRE-CREATION 
>   /trunk/proxy/proxy.properties PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyHarness.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/TestProxyClient.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/Util.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloProxy.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloSecurityException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/IOException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/KeyValueAndPeek.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/NoMoreEntriesException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PIteratorSetting.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKey.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKeyValue.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PRange.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PScanResult.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PSystemPermission.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PTablePermission.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableExistsException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableNotFoundException.java PRE-CREATION 
>   /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/UserPass.java PRE-CREATION 
>   /trunk/proxy/src/main/thrift/proxy.thrift PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyInstanceOperations.java PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyReadWrite.java PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxySecurityOperations.java PRE-CREATION 
>   /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyTableOperations.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/8039/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chris McCubbin
> 
>


Re: Review Request: Accumulo proxy source 11/13/12

Posted by Chris McCubbin <ch...@sqrrl.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8039/
-----------------------------------------------------------

(Updated Nov. 16, 2012, 4:14 p.m.)


Review request for accumulo.


Changes
-------

I added the couple of suggestions given for the last patch. Mainly performance improvements. 


Description
-------

Patch submitted by Chris McCubbin to add thrift proxy to Accumulo, second version


Diffs (updated)
-----

  /trunk/proxy/README PRE-CREATION 
  /trunk/proxy/examples/python/README PRE-CREATION 
  /trunk/proxy/examples/python/TestClient.py PRE-CREATION 
  /trunk/proxy/examples/python/proxy/AccumuloProxy-remote PRE-CREATION 
  /trunk/proxy/examples/python/proxy/AccumuloProxy.py PRE-CREATION 
  /trunk/proxy/examples/python/proxy/__init__.py PRE-CREATION 
  /trunk/proxy/examples/python/proxy/constants.py PRE-CREATION 
  /trunk/proxy/examples/python/proxy/ttypes.py PRE-CREATION 
  /trunk/proxy/examples/ruby/README PRE-CREATION 
  /trunk/proxy/examples/ruby/accumulo_proxy.rb PRE-CREATION 
  /trunk/proxy/examples/ruby/proxy_constants.rb PRE-CREATION 
  /trunk/proxy/examples/ruby/proxy_types.rb PRE-CREATION 
  /trunk/proxy/examples/ruby/test_client.rb PRE-CREATION 
  /trunk/proxy/examples/ruby/thrift.rb PRE-CREATION 
  /trunk/proxy/pom.xml PRE-CREATION 
  /trunk/proxy/proxy.properties PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyHarness.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/TestProxyClient.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/Util.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloException.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloProxy.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloSecurityException.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/IOException.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/KeyValueAndPeek.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/NoMoreEntriesException.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PIteratorSetting.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKey.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKeyValue.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PRange.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PScanResult.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PSystemPermission.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PTablePermission.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableExistsException.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableNotFoundException.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/UserPass.java PRE-CREATION 
  /trunk/proxy/src/main/thrift/proxy.thrift PRE-CREATION 
  /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyInstanceOperations.java PRE-CREATION 
  /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyReadWrite.java PRE-CREATION 
  /trunk/proxy/src/test/java/org/apache/accumulo/TestProxySecurityOperations.java PRE-CREATION 
  /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyTableOperations.java PRE-CREATION 

Diff: https://reviews.apache.org/r/8039/diff/


Testing
-------


Thanks,

Chris McCubbin


Re: Review Request: Accumulo proxy source 11/13/12

Posted by Chris McCubbin <ch...@sqrrl.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8039/
-----------------------------------------------------------

(Updated Nov. 14, 2012, 6:51 p.m.)


Review request for accumulo.


Changes
-------

This patch includes:
* Bugfixes and some class name normalization
* Ability to create Scanners in addition to BatchScanners. Reading from either scanner use the same method.
* Updated the python and ruby examples


Description
-------

Patch submitted by Chris McCubbin to add thrift proxy to Accumulo, second version


Diffs (updated)
-----

  /trunk/pom.xml 1409290 
  /trunk/proxy/README PRE-CREATION 
  /trunk/proxy/accumulo-proxy.iml PRE-CREATION 
  /trunk/proxy/examples/python/README PRE-CREATION 
  /trunk/proxy/examples/python/TestClient.py PRE-CREATION 
  /trunk/proxy/examples/python/proxy/AccumuloProxy-remote PRE-CREATION 
  /trunk/proxy/examples/python/proxy/AccumuloProxy.py PRE-CREATION 
  /trunk/proxy/examples/python/proxy/__init__.py PRE-CREATION 
  /trunk/proxy/examples/python/proxy/constants.py PRE-CREATION 
  /trunk/proxy/examples/python/proxy/ttypes.py PRE-CREATION 
  /trunk/proxy/examples/ruby/README PRE-CREATION 
  /trunk/proxy/examples/ruby/accumulo_proxy.rb PRE-CREATION 
  /trunk/proxy/examples/ruby/proxy_constants.rb PRE-CREATION 
  /trunk/proxy/examples/ruby/proxy_types.rb PRE-CREATION 
  /trunk/proxy/examples/ruby/test_client.rb PRE-CREATION 
  /trunk/proxy/examples/ruby/thrift.rb PRE-CREATION 
  /trunk/proxy/pom.xml PRE-CREATION 
  /trunk/proxy/proxy.properties PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyHarness.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/TestProxyClient.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/Util.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloException.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloProxy.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/AccumuloSecurityException.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/IOException.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/KeyValueAndPeek.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/NoMoreEntriesException.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PIteratorSetting.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKey.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PKeyValue.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PRange.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PScanResult.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PSystemPermission.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/PTablePermission.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableExistsException.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/TableNotFoundException.java PRE-CREATION 
  /trunk/proxy/src/main/java/org/apache/accumulo/proxy/api/UserPass.java PRE-CREATION 
  /trunk/proxy/src/main/thrift/proxy.thrift PRE-CREATION 
  /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyInstanceOperations.java PRE-CREATION 
  /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyReadWrite.java PRE-CREATION 
  /trunk/proxy/src/test/java/org/apache/accumulo/TestProxySecurityOperations.java PRE-CREATION 
  /trunk/proxy/src/test/java/org/apache/accumulo/TestProxyTableOperations.java PRE-CREATION 

Diff: https://reviews.apache.org/r/8039/diff/


Testing
-------


Thanks,

Chris McCubbin