On Thu, Nov 15, 2012 at 12:40 PM, Billie Rinaldi <bil...@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 >> > >> > >> >>