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: [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]
