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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]