#408 Removed use of deprecated ClientConfiguration#412
#408 Removed use of deprecated ClientConfiguration#412mikewalch merged 6 commits intoapache:masterfrom
Conversation
| * @return Zookeeper connection information for Accumulo instance | ||
| */ | ||
| String getZookeepers(); | ||
| String getZooKeepers(); |
There was a problem hiding this comment.
That's how we have referenced them in the past and how Apache ZooKeeper spells it. I can switch back but @ctubbsii might disagree :-)
There was a problem hiding this comment.
If it's public API, we should follow upstream naming conventions. Internally, it doesn't really matter, other than as best practice for developing the habit of using the trademark properly, and perhaps also for consistency.
There was a problem hiding this comment.
Probably best for another PR
| } else if (key.equals(ClientProperty.SASL_KERBEROS_SERVER_PRIMARY.getKey())) { | ||
| config.setProperty(ClientConfiguration.ClientProperty.KERBEROS_SERVER_PRIMARY, val); | ||
| } else { | ||
| config.setProperty(key, val); |
There was a problem hiding this comment.
Switch case might be cleaner for these big if statements.
There was a problem hiding this comment.
I think they both are kind of ugly. I would prefer to keep it as is.
|
|
||
| @Override | ||
| public String getZookeepers() { | ||
| public String getZooKeepers() { |
There was a problem hiding this comment.
Do we have a consistent way to refer to "zookeepers"? Camel case or one word?
There was a problem hiding this comment.
While I would prefer Zookeeper, ZooKeeper is how the project refers to itself.
keith-turner
left a comment
keith-turner
left a comment
There was a problem hiding this comment.
I noticed while reviewing this that MiniAccumuloCluster has a public API method that returns ClientConfiguration. This method should be deprecated.
|
I pushed another commit ef0c46b to fix issues with the shell. Can you take a look at it? |
|
I refactored |
|
👍 |
4 participants
Still might do a little more work on this. I am putting this up for review and to have Jenkins run ITs