gerlowskija commented on code in PR #2714:
URL: https://github.com/apache/solr/pull/2714#discussion_r1763347010
##########
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:
> ThreadLocal's have their use to communicate something across API layers
that can't or shouldn't include the extra information.
Maybe I'm misreading your comment, but are you suggesting that this POC
differs from the valid ThreadLocal use-case you described?
Here we have API layers that we don't want to be (explicitly) concerned with
the baseURL - namely SolrRequest, and SolrClient's request-y methods. So we
use the lambda/thread-local combo to tunnel the information down to where the
path building occurs. Which feels to me very much like the valid use case of
ThreadLocal's you described above?
Or maybe you're saying that this POC uses ThreadLocals in a valid/useful
way, it's just not necessary because other options are out open?
> I don't recall seeing an API embrace a ThreadLocal to accomplish something
at it's front door / prominently.
This variable can (and should) be private. That's a PoC/code-cleanup
mistake on my part that's easily remedied.
But beyond that, the thread-local **isn't** in the API anywhere - not in the
nested `BaseUrlOverride` class, not in `SolrClient`, not even of any method in
`ClientUtils`. Can you elaborate a bit on how the API is "embracing" the
thread-local 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]
