[jira] [Comment Edited] (SOLR-12595) CloudSolrClient.Builder should accept a zkHost connection string

2018-08-01 Thread Vilius Pranckaitis (JIRA)


[ 
https://issues.apache.org/jira/browse/SOLR-12595?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16565050#comment-16565050
 ] 

Vilius Pranckaitis edited comment on SOLR-12595 at 8/1/18 9:41 AM:
---

When you use other tools:
 - You can construct a [Zookeeper 
client|https://zookeeper.apache.org/doc/r3.4.13/api/index.html] with a ZK 
connection string;
 - You can initialize a [Curator 
framework|https://curator.apache.org/apidocs/org/apache/curator/framework/CuratorFrameworkFactory.html]
 with a ZK connection string;
 - You can [launch 
Solr|https://lucene.apache.org/solr/guide/7_4/setting-up-an-external-zookeeper-ensemble.html#updating-solr-s-include-files]
 using a ZK connection string.
 - And so on...

But when it comes to initialization of {{CloudSolrClient}} using a ZK 
connection string, the options are:
 - To write some code to split ZK connection string into pieces and call the 
existing constructor;
 - To use deprecated {{withClusterStateProvider()}};
 - [To extend the 
{{CloudSolrClient.Builder}}|https://github.com/apache/lucene-solr/blob/5d7df7b0d1b122976a10cf05ace13a9ad6e1/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java#L1553-L1554].

In my opinion, the current constructors for {{CloudSolrClient.Builder}} goes 
against the principle of least astonishment. When I received a PR which had a 
comment about {{CloudSolrClient}} not supporting ZK connection strings, I 
couldn't believe and had to double-check with the Javadocs.

I'm not sure which version of the constructor–a connection string one or the 
list of hosts one–would be more confusing for someone who is not familiar with 
ZK. But for me, who had some experience with ZK – definitely the latter one.

--

I had taken a look at the recent changes of {{CloudSolrClient.Builder}} and I 
think I see where things are going. Moving some of the parameters to 
constructors will prevent some of the runtime errors, and should also allow to 
remove some of that [state provider construction 
code|https://github.com/apache/lucene-solr/blob/5d7df7b0d1b122976a10cf05ace13a9ad6e1/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java#L1567-L1576],
 which is also duplicated in 
[{{CloudSolrClient}}|https://github.com/apache/lucene-solr/blob/5d7df7b0d1b122976a10cf05ace13a9ad6e1/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java#L255-L266]
 (though probably not before some of the deprecated methods will be removed). I 
could help with this, too.


was (Author: vpranckaitis):
When you use other tools:
 - You can construct a [Zookeeper 
client|https://zookeeper.apache.org/doc/r3.4.13/api/index.html] with a ZK 
connection string;
 - You can initialize a [Curator 
framework|https://curator.apache.org/apidocs/org/apache/curator/framework/CuratorFrameworkFactory.html]
 with a ZK connection string;
 - You can [launch 
Solr|https://lucene.apache.org/solr/guide/7_4/setting-up-an-external-zookeeper-ensemble.html#updating-solr-s-include-files]
 using a ZK connection string.
 - And so on...

But when it comes to initialization of {{CloudSolrClient}} using a ZK 
connection string, the options are:
 - Write some code to split ZK connection string into pieces and call the 
existing constructor;
 - Use deprecated {{withClusterStateProvider()}};
 - [Extend the 
{{CloudSolrClient.Builder}}|https://github.com/apache/lucene-solr/blob/5d7df7b0d1b122976a10cf05ace13a9ad6e1/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java#L1553-L1554].

In my opinion, the current constructors for {{CloudSolrClient.Builder}} goes 
against the principle of least astonishment. When I received a PR which had a 
comment about {{CloudSolrClient}} not supporting ZK connection strings, I 
couldn't believe and had to double-check with the Javadocs.

I'm not sure which version of the constructor–a connection string one or the 
list of hosts one–would be more confusing for someone who is not familiar with 
ZK. But for me, who had some experience with ZK – definitely the latter one.

--

I had taken a look at the recent changes of {{CloudSolrClient.Builder}} and I 
think I see where things are going. Moving some of the parameters to 
constructors will prevent some of the runtime errors, and should also allow to 
remove some of that [state provider construction 
code|https://github.com/apache/lucene-solr/blob/5d7df7b0d1b122976a10cf05ace13a9ad6e1/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java#L1567-L1576],
 which is also duplicated in 
[{{CloudSolrClient}}|https://github.com/apache/lucene-solr/blob/5d7df7b0d1b122976a10cf05ace13a9ad6e1/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java#L255-L266]
 (though probably not before some of the deprecated methods will be removed). I 
could help with this, too.

> CloudSolrClient.Builder s

[jira] [Commented] (SOLR-12595) CloudSolrClient.Builder should accept a zkHost connection string

2018-08-01 Thread Vilius Pranckaitis (JIRA)


[ 
https://issues.apache.org/jira/browse/SOLR-12595?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16565050#comment-16565050
 ] 

Vilius Pranckaitis commented on SOLR-12595:
---

When you use other tools:
 - You can construct a [Zookeeper 
client|https://zookeeper.apache.org/doc/r3.4.13/api/index.html] with a ZK 
connection string;
 - You can initialize a [Curator 
framework|https://curator.apache.org/apidocs/org/apache/curator/framework/CuratorFrameworkFactory.html]
 with a ZK connection string;
 - You can [launch 
Solr|https://lucene.apache.org/solr/guide/7_4/setting-up-an-external-zookeeper-ensemble.html#updating-solr-s-include-files]
 using a ZK connection string.
 - And so on...

But when it comes to initialization of {{CloudSolrClient}} using a ZK 
connection string, the options are:
 - Write some code to split ZK connection string into pieces and call the 
existing constructor;
 - Use deprecated {{withClusterStateProvider()}};
 - [Extend the 
{{CloudSolrClient.Builder}}|https://github.com/apache/lucene-solr/blob/5d7df7b0d1b122976a10cf05ace13a9ad6e1/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java#L1553-L1554].

In my opinion, the current constructors for {{CloudSolrClient.Builder}} goes 
against the principle of least astonishment. When I received a PR which had a 
comment about {{CloudSolrClient}} not supporting ZK connection strings, I 
couldn't believe and had to double-check with the Javadocs.

I'm not sure which version of the constructor–a connection string one or the 
list of hosts one–would be more confusing for someone who is not familiar with 
ZK. But for me, who had some experience with ZK – definitely the latter one.

--

I had taken a look at the recent changes of {{CloudSolrClient.Builder}} and I 
think I see where things are going. Moving some of the parameters to 
constructors will prevent some of the runtime errors, and should also allow to 
remove some of that [state provider construction 
code|https://github.com/apache/lucene-solr/blob/5d7df7b0d1b122976a10cf05ace13a9ad6e1/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java#L1567-L1576],
 which is also duplicated in 
[{{CloudSolrClient}}|https://github.com/apache/lucene-solr/blob/5d7df7b0d1b122976a10cf05ace13a9ad6e1/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java#L255-L266]
 (though probably not before some of the deprecated methods will be removed). I 
could help with this, too.

> CloudSolrClient.Builder should accept a zkHost connection string
> 
>
> Key: SOLR-12595
> URL: https://issues.apache.org/jira/browse/SOLR-12595
> Project: Solr
>  Issue Type: Improvement
>  Security Level: Public(Default Security Level. Issues are Public) 
>  Components: SolrJ
>Affects Versions: 7.4
>Reporter: Vilius Pranckaitis
>Priority: Minor
>  Time Spent: 20m
>  Remaining Estimate: 0h
>
> SOLR-11629 improved {{CloudSolrClient.Builder}} workflow by adding two new 
> constructors:
> {code:java}
> 1.   public Builder(List solrUrls) {
> 2.   public Builder(List zkHosts, Optional zkChroot) {
> {code}
> It is not unusual to format ZooKeeper connection details as a single string 
> (e.g. {{zk1:2181,zk2:2181/some_chroot}}). However, these new constructors 
> make it difficult to use such connection strings.
> It would be fairly simple to add a third constructor which would accept a 
> connection string:
> {code:java}
> 3.   public Builder(String zkHost) {
> {code}
> {{CloudSolrClient.Builder}} uses ZooKeeper details to construct a 
> {{ZkClientClusterStateProvider}}, which [already supports ZK connection 
> strings|https://github.com/apache/lucene-solr/blob/d87ea6b1ccd28e0dd8e30565fe95b2e0a31f82e8/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ZkClientClusterStateProvider.java#L57-L59].



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org



[jira] [Updated] (SOLR-12595) CloudSolrClient.Builder should accept a zkHost connection string

2018-07-27 Thread Vilius Pranckaitis (JIRA)


 [ 
https://issues.apache.org/jira/browse/SOLR-12595?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Vilius Pranckaitis updated SOLR-12595:
--
Component/s: SolrJ

> CloudSolrClient.Builder should accept a zkHost connection string
> 
>
> Key: SOLR-12595
> URL: https://issues.apache.org/jira/browse/SOLR-12595
> Project: Solr
>  Issue Type: Improvement
>  Security Level: Public(Default Security Level. Issues are Public) 
>  Components: SolrJ
>Affects Versions: 7.4
>Reporter: Vilius Pranckaitis
>Priority: Minor
>
> SOLR-11629 improved {{CloudSolrClient.Builder}} workflow by adding two new 
> constructors:
> {code:java}
> 1.   public Builder(List solrUrls) {
> 2.   public Builder(List zkHosts, Optional zkChroot) {
> {code}
> It is not unusual to format ZooKeeper connection details as a single string 
> (e.g. {{zk1:2181,zk2:2181/some_chroot}}). However, these new constructors 
> make it difficult to use such connection strings.
> It would be fairly simple to add a third constructor which would accept a 
> connection string:
> {code:java}
> 3.   public Builder(String zkHost) {
> {code}
> {{CloudSolrClient.Builder}} uses ZooKeeper details to construct a 
> {{ZkClientClusterStateProvider}}, which [already supports ZK connection 
> strings|https://github.com/apache/lucene-solr/blob/d87ea6b1ccd28e0dd8e30565fe95b2e0a31f82e8/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ZkClientClusterStateProvider.java#L57-L59].



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org



[jira] [Updated] (SOLR-12595) CloudSolrClient.Builder should accept a zkHost connection string

2018-07-27 Thread Vilius Pranckaitis (JIRA)


 [ 
https://issues.apache.org/jira/browse/SOLR-12595?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Vilius Pranckaitis updated SOLR-12595:
--
Affects Version/s: 7.4

> CloudSolrClient.Builder should accept a zkHost connection string
> 
>
> Key: SOLR-12595
> URL: https://issues.apache.org/jira/browse/SOLR-12595
> Project: Solr
>  Issue Type: Improvement
>  Security Level: Public(Default Security Level. Issues are Public) 
>  Components: SolrJ
>Affects Versions: 7.4
>Reporter: Vilius Pranckaitis
>Priority: Minor
>
> SOLR-11629 improved {{CloudSolrClient.Builder}} workflow by adding two new 
> constructors:
> {code:java}
> 1.   public Builder(List solrUrls) {
> 2.   public Builder(List zkHosts, Optional zkChroot) {
> {code}
> It is not unusual to format ZooKeeper connection details as a single string 
> (e.g. {{zk1:2181,zk2:2181/some_chroot}}). However, these new constructors 
> make it difficult to use such connection strings.
> It would be fairly simple to add a third constructor which would accept a 
> connection string:
> {code:java}
> 3.   public Builder(String zkHost) {
> {code}
> {{CloudSolrClient.Builder}} uses ZooKeeper details to construct a 
> {{ZkClientClusterStateProvider}}, which [already supports ZK connection 
> strings|https://github.com/apache/lucene-solr/blob/d87ea6b1ccd28e0dd8e30565fe95b2e0a31f82e8/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ZkClientClusterStateProvider.java#L57-L59].



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org



[jira] [Commented] (SOLR-12595) CloudSolrClient.Builder should accept a zkHost connection string

2018-07-26 Thread Vilius Pranckaitis (JIRA)


[ 
https://issues.apache.org/jira/browse/SOLR-12595?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16558264#comment-16558264
 ] 

Vilius Pranckaitis commented on SOLR-12595:
---

There's a workaround, but you need to call at least one {{@Deprecated}} method: 
you could construct {{ZkClientClusterStateProvider}} yourself and pass it using 
[{{withClusterStateProvider()}}|https://github.com/apache/lucene-solr/blob/d87ea6b1ccd28e0dd8e30565fe95b2e0a31f82e8/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java#L1556-L1559]
 method.

I'm volunteering to implement this new constructor.

> CloudSolrClient.Builder should accept a zkHost connection string
> 
>
> Key: SOLR-12595
> URL: https://issues.apache.org/jira/browse/SOLR-12595
> Project: Solr
>  Issue Type: Improvement
>  Security Level: Public(Default Security Level. Issues are Public) 
>Reporter: Vilius Pranckaitis
>Priority: Minor
>
> SOLR-11629 improved {{CloudSolrClient.Builder}} workflow by adding two new 
> constructors:
> {code:java}
> 1.   public Builder(List solrUrls) {
> 2.   public Builder(List zkHosts, Optional zkChroot) {
> {code}
> It is not unusual to format ZooKeeper connection details as a single string 
> (e.g. {{zk1:2181,zk2:2181/some_chroot}}). However, these new constructors 
> make it difficult to use such connection strings.
> It would be fairly simple to add a third constructor which would accept a 
> connection string:
> {code:java}
> 3.   public Builder(String zkHost) {
> {code}
> {{CloudSolrClient.Builder}} uses ZooKeeper details to construct a 
> {{ZkClientClusterStateProvider}}, which [already supports ZK connection 
> strings|https://github.com/apache/lucene-solr/blob/d87ea6b1ccd28e0dd8e30565fe95b2e0a31f82e8/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ZkClientClusterStateProvider.java#L57-L59].



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org



[jira] [Created] (SOLR-12595) CloudSolrClient.Builder should accept a zkHost connection string

2018-07-26 Thread Vilius Pranckaitis (JIRA)
Vilius Pranckaitis created SOLR-12595:
-

 Summary: CloudSolrClient.Builder should accept a zkHost connection 
string
 Key: SOLR-12595
 URL: https://issues.apache.org/jira/browse/SOLR-12595
 Project: Solr
  Issue Type: Improvement
  Security Level: Public (Default Security Level. Issues are Public)
Reporter: Vilius Pranckaitis


SOLR-11629 improved {{CloudSolrClient.Builder}} workflow by adding two new 
constructors:
{code:java}
1.   public Builder(List solrUrls) {
2.   public Builder(List zkHosts, Optional zkChroot) {
{code}
It is not unusual to format ZooKeeper connection details as a single string 
(e.g. {{zk1:2181,zk2:2181/some_chroot}}). However, these new constructors make 
it difficult to use such connection strings.

It would be fairly simple to add a third constructor which would accept a 
connection string:
{code:java}
3.   public Builder(String zkHost) {
{code}
{{CloudSolrClient.Builder}} uses ZooKeeper details to construct a 
{{ZkClientClusterStateProvider}}, which [already supports ZK connection 
strings|https://github.com/apache/lucene-solr/blob/d87ea6b1ccd28e0dd8e30565fe95b2e0a31f82e8/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ZkClientClusterStateProvider.java#L57-L59].



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org