Update python client for latest accumulo-proxy and accumulo:2.1.1 #5
Conversation
…core to match. Add dockerfile for testing in cluster.
magrable
changed the title
There was a problem hiding this comment.
Honestly, I'm a bit surprised this didn't require more extensive changes with the changes that are happening with accumulo-proxy.
Since accumulo-proxy has not yet been released for use with Accumulo 2.1, you must've used a snapshot version. Do you know which git commit that was? I'm just curious if there's been any substantive changes since this was last tested.
| ``` | ||
|
|
||
| __Note__. This library is a work in progress. It has been tested with Accumulo 1.9 and Python 3.8. | ||
| __Note__. This library is a work in progress. It has been tested with Accumulo 2.1.1 and Python 3.8. |
There was a problem hiding this comment.
While there may have been testing done with Accumulo 2.1.1, the accumulo-proxy has not yet had a released version that works with 2.1. The sharedSecret behavior, for example, only exists in the unreleased SNAPSHOT version.
I think this notice could be more clear that the accumulo-proxy this project relies on may itself still be under development, but I'm not sure exactly how to phrase that.
There was a problem hiding this comment.
Looking at the open issues against the proxy - is there anything holding up a release that works with 2.1? I'm not sure when the last release was, and I don't see any tags at all since the proxy was forked to it's own repository. Fwiw, we've been running with this apache/accumulo-proxy#81 against various accumulo 2.1.X clusters and things seem to be working as expected.
| sharedSecret = "sharedSecret" | ||
| context = AccumuloProxyConnectionContext(proxy_connection) | ||
| connector = context.create_connector('user', 'secret') | ||
| connector = context.create_connector(sharedSecret) |
There was a problem hiding this comment.
The underlying proxy uses a configuration file to set up an AccumuloClient object. It is no longer necessary for proxy client code to create a connector, and there is no mechanism to do so in the newer proxy code. So, I'm not sure this create connector makes sense anymore.
| ```python | ||
| # executor.run(gettern_fn, *args) | ||
| await executor.run(lambda c: c.tableExists, login, 'tmp') | ||
| await executor.run(lambda c: c.tableExists, 'sharedSecret', 'tmp') |
There was a problem hiding this comment.
In these examples, the shared secret is being provided as a string, but probably should show an example of passing it as a variable, the same way the login variable was previously passed. Users are probably going to want to save that in a variable, rather than type it every time, or have it as a string literal everywhere.
| await executor.run(lambda c: c.tableExists, 'sharedSecret', 'tmp') | |
| await executor.run(lambda c: c.tableExists, sharedSecret, 'tmp') |
| login = await self.proxy_connection_pool_executor.run(AccumuloProxyClientFunctionGetters.login, user, | ||
| {'password': password}) | ||
| return AsyncAccumuloConnector(self.proxy_connection_pool_executor, login) | ||
| async def create_connector(self, shared_secret: bytes) -> AsyncAccumuloConnector: |
There was a problem hiding this comment.
With Accumulo 2.x, we deprecated the "Connector" APIs, and that terminology, in favor of the more familiar "Client" terminology (and APIs). I don't know if it makes sense to continue calling this python library's methods and types "connector" or if it makes sense to rename them to match to the corresponding Accumulo types.
Then again, the word "client" is very overloaded here... because you have proxy, which is a client to Accumulo and has an AccumuloClient object, but is also a Thrift service, and there is the generated Thrift client code that is called by this library. Then, there is client code to this library. It is a bit confusing. Perhaps this library may be able to choose a different name that helps disambiguate them, for users of this library, at least?
setup.py
Outdated
| description='Build Python 3 applications that integrate with Apache Accumulo', | ||
| install_requires=[ | ||
| 'thrift>=0.13.0' | ||
| 'thrift>=0.16.0' # Update to thrift 0.17.0, pending https://issues.apache.org/jira/projects/THRIFT/issues/THRIFT-5546 |
There was a problem hiding this comment.
| 'thrift>=0.16.0' # Update to thrift 0.17.0, pending https://issues.apache.org/jira/projects/THRIFT/issues/THRIFT-5546 | |
| 'thrift>=0.16.0' # Update to thrift 0.17.0, pending https://issues.apache.org/jira/browse/THRIFT-5688 |
|
I used pip to build and install the accumulo-python3 client from b68900f into a Python 3.10 virtualenv created using Anaconda. I ran a quick test using the code in the README combined with a built snapshot of the accumulo-proxy from The test code completed sucesfuly, and I validated that the rows were present in the tmp table via the Accumulo shell. |
3 participants
Update accumulo.thrift to latest accumulo-proxy generated thrift bindings.
Update accumulo.core to match.
Add dockerfile for testing in cluster.