Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Accumulo, mail # dev - Review Request 14972: ACCUMULO-1009 - add use of SSL for thrift comms


Copy link to this message
-
Re: Review Request 14972: ACCUMULO-1009 - add use of SSL for thrift comms
John Vines 2013-10-31, 18:23


> On Oct. 29, 2013, 3:25 p.m., kturner wrote:
> > core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java, line 102
> > <https://reviews.apache.org/r/14972/diff/1/?file=371853#file371853line102>
> >
> >     I think I advocated somewhere for removing this method, because its not scaleable for every feature to have a seat at the table in the constructor.  The constructor that takes the config object nicely covers this use case.
>
> John Vines wrote:
>     I was under the impression that this fell under ACCUMULO-1726
>
> kturner wrote:
>     How is adding a redundant method to the API now related to 1726?
>
> John Vines wrote:
>     Misunderstood, agree with your assessment.
>
> Michael Berman wrote:
>     I did remove most of the constructors I added, and I agree it's better without the bloat, but I am a little concerned about discoverability.  I guess it depends what our long-term plan is.  If we're expecting that everyone will always use the single-argument ClientConfiguration constructor, then maybe we should start deprecating the others (and then discoverability is less of a concern because people will see the options on ClientConfiguration).  But if we think (String instanceName, String zooKeepers) will continue to be the usual constructor, then I think it does make sense to have the most common options available in a variant of that one (although I think it's good not to have every possible permutation of switches cluttering up the interface).
>
> Sean Busbey wrote:
>     I think pushing towards the ClientConfiguration constructor is the way to go. I see an argument for keeping the very simple (String instanceName, String zooKeepers) for examples, but I'm +0 on it.

Personally, I'm in favor of deprecating ZooKeeperInstance(String instanceName, String zooKeepers) for the new ZooKeeperInstance(ClientConfiguration)
> On Oct. 29, 2013, 3:25 p.m., kturner wrote:
> > core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java, line 164
> > <https://reviews.apache.org/r/14972/diff/1/?file=371853#file371853line164>
> >
> >     Probably do not need to add this method.  I think most users connect using the instance name.  If they really want to connect using the instance id and pass config, can they use the constructor that only takes config?
>
> John Vines wrote:
>     Unless we decide to entirely drop support for the UUID based constructors, then we should have them mirror any additional functionality.
>
> kturner wrote:
>     Having ZooKeeperInstance(ClientConfiguration) changes things.  I am assuming this constructor can replicate the functionality of all the other existing constructors, is this correct?  I'm thinking we should be moving towards less constructors if ZooKeeperInstance(ClientConfiguration) exist and not more.
>
> John Vines wrote:
>     Agreed
>
> Michael Berman wrote:
>     I think this makes sense, but this relates to my question above about what we think the usual entrypoint will be and if we should start deprecating all the others.  If we really think the single-argument ClientConfiguration constructor is the best way to make a ZKInstance, then we should start encouraging people to migrate to it, right?

Yes
- John
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14972/#review27675
-----------------------------------------------------------
On Oct. 31, 2013, 2:35 p.m., John Vines wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14972/
> -----------------------------------------------------------
>
> (Updated Oct. 31, 2013, 2:35 p.m.)
>
>
> Review request for accumulo and Michael Berman.
>
>
> Bugs: ACCUMULO-1009
>     https://issues.apache.org/jira/browse/ACCUMULO-1009
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> Michael Berman's October 13 patch for ACCUMULO-1009