Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parse GRPC data in ZooKeeper. #14

Merged
merged 1 commit into from
Sep 10, 2019
Merged

Conversation

vexingcodes
Copy link
Contributor

  1. Install kazoo (ZooKeeper client for Python)
  2. Install GRPC and GRPC tools (protobuf compiler)
  3. Compile all of the RAMCloud GRPC proto files for python as part of ./config/make-ramcloud
  4. Add simple test to read a GRPC value from ZooKeeper and parse it.

external_storage = ctu.external_storage_string(ensemble)
self.ensemble = {i: '10.0.1.{}'.format(i) for i in xrange(1, num_nodes + 1)}
zk_servers = ctu.ensemble_servers_string(self.ensemble)
external_storage = 'zk:' + ctu.external_storage_string(self.ensemble)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that, with your change, some calls need the 'zk:' prefix, and other calls do not. but moving the 'zk:' string over to this file for concatenation feels a bit tacky. Wouldn't it be cleaner to simply add a boolean parameter use_zk_prefix for the ctu.external_storage_string() method?

# Read the ZooKeeper entry for the table and make sure it looks sane.
# This mostly tests our ability to read from ZooKeeper and parse the
# GRPC contents correctly.
table_data = zk_client.get('/ramcloud/main/tables/test')[0]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely sure why you're using '/ramcloud/main/tables/test' for table_data. I'm sure there's a good reason. Can I ask you to clarify this in the comment?

@vexingcodes vexingcodes merged commit cef37dd into BeStateless:master Sep 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants