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 available? > 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: 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