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

Reply via email to