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


##########
solr/solrj/src/java/org/apache/solr/client/solrj/util/ClientUtils.java:
##########
@@ -102,6 +111,24 @@ public static String buildRequestUrl(
     return basePath + path;
   }
 
+  /**
+   * Syntactic-sugar to set/unset {@link ClientUtils#baseUrlOverride} via 
try-with-resources blocks
+   *
+   * <p>Instances should not be used across threads, and <em>must</em> be 
closed by the same thread
+   * that opened them
+   */
+  public static class BaseUrlOverride implements Closeable {
+    public BaseUrlOverride(String baseUrl) {
+      baseUrlOverride.set(baseUrl);
+    }
+
+    public void close() {
+      baseUrlOverride.remove();
+    }
+  }
+
+  public static final ThreadLocal<String> baseUrlOverride = new 
ThreadLocal<>();

Review Comment:
   Use of a ThreadLocal is pretty weird here IMO.  ThreadLocal's have their use 
to communicate something across API layers that can't or shouldn't include the 
extra information.  That doesn't apply here; a requestWithUri would work 
without such a hack.  I don't recall seeing an API embrace a ThreadLocal to 
accomplish something at it's front door / prominently.



-- 
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-unsubscr...@solr.apache.org

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


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

Reply via email to