You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "dsmiley (via GitHub)" <gi...@apache.org> on 2023/02/13 03:53:05 UTC

[GitHub] [solr] dsmiley commented on a diff in pull request #1338: SOLR-16595: Standardize Solr Client Builders handling of times

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


##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientBuilder.java:
##########
@@ -101,13 +116,27 @@ public B withConnectionTimeout(int connectionTimeoutMillis) {
    * sockets.
    *
    * <p>For valid values see {@link org.apache.http.client.config.RequestConfig#getSocketTimeout()}
+   *
+   * <p>* @deprecated Please use {@link #withSocketTimeout(long, TimeUnit)}
    */
+  @Deprecated(since = "9.2")
   public B withSocketTimeout(int socketTimeoutMillis) {
+    withSocketTimeout(connectionTimeoutMillis, TimeUnit.MILLISECONDS);

Review Comment:
   you aren't referring to the parameter; this is why the test failed.  IntelliJ made this easy to spot; showed the parameter in gray.



##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientBuilder.java:
##########
@@ -101,13 +116,27 @@ public B withConnectionTimeout(int connectionTimeoutMillis) {
    * sockets.
    *
    * <p>For valid values see {@link org.apache.http.client.config.RequestConfig#getSocketTimeout()}
+   *
+   * <p>* @deprecated Please use {@link #withSocketTimeout(long, TimeUnit)}
    */
+  @Deprecated(since = "9.2")
   public B withSocketTimeout(int socketTimeoutMillis) {
+    withSocketTimeout(connectionTimeoutMillis, TimeUnit.MILLISECONDS);
+    return getThis();
+  }
+
+  /**
+   * Tells {@link Builder} that created clients should set the following read timeout on all
+   * sockets.
+   *
+   * <p>For valid values see {@link org.apache.http.client.config.RequestConfig#getSocketTimeout()}
+   */
+  public B withSocketTimeout(long socketTimeout, TimeUnit unit) {
     if (socketTimeoutMillis < 0) {

Review Comment:
   again you aren't referring to the parameter



##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientBuilder.java:
##########
@@ -86,13 +87,27 @@ public B withFollowRedirects(boolean followRedirects) {
    * Solr servers.
    *
    * <p>For valid values see {@link org.apache.http.client.config.RequestConfig#getConnectTimeout()}
+   *
+   * @deprecated Please use {@link #withConnectionTimeout(long, TimeUnit)}
    */
+  @Deprecated(since = "9.2")
   public B withConnectionTimeout(int connectionTimeoutMillis) {
+    withConnectionTimeout(connectionTimeoutMillis, TimeUnit.MILLISECONDS);
+    return getThis();
+  }
+
+  /**
+   * Tells {@link Builder} that created clients should obey the following timeout when connecting to
+   * Solr servers.
+   *
+   * <p>For valid values see {@link org.apache.http.client.config.RequestConfig#getConnectTimeout()}
+   */
+  public B withConnectionTimeout(long connectionTimeout, TimeUnit unit) {
     if (connectionTimeoutMillis < 0) {

Review Comment:
   I suspect you had this problem because you renamed the parameter manually (as if your IDE was a simple text editor) to remove the "Millis" instead of telling your IDE (IntelliJ) to refactor-rename the parameter.  By far this is one of the most common IDE functions I do -- rename.  Memorize the hotkey.  I think recent IntelliJ versions even have some sort of after-the-fact detection of this to do the rename if you simply edit but it's not as reliable.



##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientBuilder.java:
##########
@@ -86,13 +87,27 @@ public B withFollowRedirects(boolean followRedirects) {
    * Solr servers.
    *
    * <p>For valid values see {@link org.apache.http.client.config.RequestConfig#getConnectTimeout()}
+   *
+   * @deprecated Please use {@link #withConnectionTimeout(long, TimeUnit)}
    */
+  @Deprecated(since = "9.2")
   public B withConnectionTimeout(int connectionTimeoutMillis) {
+    withConnectionTimeout(connectionTimeoutMillis, TimeUnit.MILLISECONDS);
+    return getThis();
+  }
+
+  /**
+   * Tells {@link Builder} that created clients should obey the following timeout when connecting to
+   * Solr servers.
+   *
+   * <p>For valid values see {@link org.apache.http.client.config.RequestConfig#getConnectTimeout()}
+   */
+  public B withConnectionTimeout(long connectionTimeout, TimeUnit unit) {
     if (connectionTimeoutMillis < 0) {

Review Comment:
   you aren't referring to the parameter



-- 
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