Skip to content

Conversation

@lstav
Copy link
Contributor

@lstav lstav commented Jan 6, 2017

Fixes ACCUMULO-4061. I started working on the shell code on a separate branch, which is why I removed the commits done for #ACCUMULO-4558 from this branch and moved them to the new one. If you wish to see what the commits comments were, you can look at them from the other branch (the commits will have ACCUMULO-4061 in the message).

@joshelser
Copy link
Member

Yes, it'd be nice if the other services also had a status API (with a version).

+1.

Having a version for the Master RPC would also be good.

Have you thought about trying to write a test for this? :) It would be a good adventure for you in using our lower-level RPC API calls to interact with Accumulo (instead of just the public API). Can point you in the right direction for examples if you're amenable

@lstav
Copy link
Contributor Author

lstav commented Jan 11, 2017

It would be interesting to see and try to write a test for it. I would appreciate if you point me in the right direction.

@joshelser
Copy link
Member

joshelser commented Jan 11, 2017

It would be interesting to see and try to write a test for it. I would appreciate if you point me in the right direction.

You've had a little bit of experience here, seeing what Thrift generates and how to implement the server-side interface.

At a high level, think about the Accumulo Java 'Public API' is a wrapper around the Thrift RPCs. Client-side, we also have the Thrift generated code. Typically, the public API call would call either MasterClient.execute(..), ServerClient.execute(..), or ThriftUtil.getTServerClient(..). These calls are for Master RPCs, RPC to any TabletServer, or RPC to a specific TabletServer, respectively.

I would not worry about doing a low-level test case, instead, build some public API method and then write a test around that new method. I think adding a method to InstanceOperations makes the most sense here.

If we decide that we don't want to make a new public API call for this, using MasterClient.execute to invoke the getMasterStats(..) method.

MasterClient.executeVoid(context, new ClientExec<MasterClientService.Client>() {
@Override
public void execute(MasterClientService.Client client) throws Exception {
client.setSystemProperty(Tracer.traceInfo(), context.rpcCreds(), property, value);
}
});
is an example of how to invoke this method. Hopefully this will be very straightforward to do with any of the existing test classes that extend AccumuloClusterIT.

@lstav
Copy link
Contributor Author

lstav commented Jan 26, 2017

Closing since the changes on this pull request are in #200

@lstav lstav closed this Jan 26, 2017
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.

3 participants