gerlowskija commented on code in PR #2473: URL: https://github.com/apache/solr/pull/2473#discussion_r1611983813
########## solr/solrj/src/java/org/apache/solr/common/util/Utils.java: ########## @@ -716,10 +716,30 @@ public static String applyUrlScheme(final String url, final String urlScheme) { return (at == -1) ? (urlScheme + "://" + url) : urlScheme + url.substring(at); } + /** Review Comment: [+1] Awesome Javadocs, thanks for adding these. [0] Definitely not a job for this PR, but I think we could make SolrJ easier to understand if we were a bit tighter in our URL/path terminology. "Base" means a lot of different things to a lot of different people. You could imagine defining terms like "root URL" = `scheme://host:port/$contextRoot`, "API base URL" = `$root/api` (v2), `$root/solr` (v1), "collection URL" = ... Just thinking aloud. This is all very minor, but I've spent a lot of time lately looking at how SolrJ builds these APIs so it's on my mind. ########## solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java: ########## @@ -1590,8 +1590,7 @@ private long uploadGivenConfigSet( Map<?, ?> map = postDataAndGetResponse( cluster.getSolrClient(), - cluster.getJettySolrRunners().get(0).getBaseUrl().toString().replace("/solr", "") - + uriEnding, + cluster.getJettySolrRunners().get(0).getBaseURLV2().toString() + uriEnding, Review Comment: > Just wondering if it does it implicitly. It doesn't 😦 @epugh I agree with what I think is your implicit point (and with a point you made explicitly on one of my recent PRs haha) - that we should add a variant of these "getUrl" methods that returns a string instead of a URL, since this is such a common idiom. But let's handle that as a separate PR. -- 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