You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/12/11 06:25:48 UTC

[GitHub] [solr] dsmiley commented on a diff in pull request #1220: SOLR-10463: setRetryExpiryTime should be deprecated in favor of Solr Client Builder methods

dsmiley commented on code in PR #1220:
URL: https://github.com/apache/solr/pull/1220#discussion_r1045179129


##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientBuilder.java:
##########
@@ -34,6 +35,8 @@
   protected boolean useMultiPartPost;
   protected Integer connectionTimeoutMillis = 15000;
   protected Integer socketTimeoutMillis = 120000;
+  protected long retryExpiryTime =

Review Comment:
   SolrClientBuilder is for all the Apache HttpClient based SolrClients, but retryExpiryTime is specifically for ZK.  It's wrong to put here.



##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java:
##########
@@ -245,6 +249,16 @@ public Builder withParallelUpdates(boolean parallelUpdates) {
       return this;
     }
 
+    /**
+     * This is the time to wait to refetch the state after getting the same state version from ZK
+     *
+     * <p>secs
+     */
+    public Builder setRetryExpiryTime(int secs) {

Review Comment:
   Can you incorporate TimeUnit as a parameter so that it's clearer what the unit is at the call site?
   IntelliJ tip: do this by selecting "TimeUnit.SECONDS" on the line below and doing cmd-opt-P (introduce parameter)



##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java:
##########
@@ -245,6 +249,16 @@ public Builder withParallelUpdates(boolean parallelUpdates) {
       return this;
     }
 
+    /**
+     * This is the time to wait to refetch the state after getting the same state version from ZK
+     *
+     * <p>secs
+     */
+    public Builder setRetryExpiryTime(int secs) {

Review Comment:
   No "with" terminology?



##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java:
##########
@@ -245,6 +249,16 @@ public Builder withParallelUpdates(boolean parallelUpdates) {
       return this;
     }
 
+    /**
+     * This is the time to wait to refetch the state after getting the same state version from ZK
+     *
+     * <p>secs

Review Comment:
   Delete, I think



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org