Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SOLR-17213: Make warning optional so we can warn only when solrUrl is user entered #2377

Merged
merged 4 commits into from
Apr 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions solr/CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,8 @@ Bug Fixes
* SOLR-17206: Eliminate the possibility of a -1 status code for SolrCloud update requests that are distributed.
(Paul McArthur)

* SOLR-17213: Fix spurious warnings about solr url format in Solr CLI when users aren't providing a deprecated solr url. (Eric Pugh)

Dependency Upgrades
---------------------

Expand Down
27 changes: 21 additions & 6 deletions solr/core/src/java/org/apache/solr/cli/SolrCLI.java
Original file line number Diff line number Diff line change
Expand Up @@ -505,15 +505,29 @@ private static void printHelp() {
* @return the solrUrl in the format that Solr expects to see internally.
*/
public static String normalizeSolrUrl(String solrUrl) {
return normalizeSolrUrl(solrUrl, true);
}

/**
* Strips off the end of solrUrl any /solr when a legacy solrUrl like http://localhost:8983/solr
* is used, and optionally logs a warning. In the future we'll have urls ending with /api as well.
*
* @param solrUrl The user supplied url to Solr.
* @param logUrlFormatWarning If a warning message should be logged about the url format
* @return the solrUrl in the format that Solr expects to see internally.
*/
public static String normalizeSolrUrl(String solrUrl, boolean logUrlFormatWarning) {
if (solrUrl != null) {
if (solrUrl.contains("/solr")) { //
String newSolrUrl = solrUrl.substring(0, solrUrl.indexOf("/solr"));
CLIO.out(
"WARNING: URLs provided to this tool needn't include Solr's context-root (e.g. \"/solr\"). Such URLs are deprecated and support for them will be removed in a future release. Correcting from ["
+ solrUrl
+ "] to ["
+ newSolrUrl
+ "].");
if (logUrlFormatWarning) {
CLIO.out(
"WARNING: URLs provided to this tool needn't include Solr's context-root (e.g. \"/solr\"). Such URLs are deprecated and support for them will be removed in a future release. Correcting from ["
+ solrUrl
+ "] to ["
+ newSolrUrl
+ "].");
}
solrUrl = newSolrUrl;
}
if (solrUrl.endsWith("/")) {
Expand Down Expand Up @@ -551,6 +565,7 @@ public static String normalizeSolrUrl(CommandLine cli) throws Exception {

String firstLiveNode = liveNodes.iterator().next();
solrUrl = ZkStateReader.from(cloudSolrClient).getBaseUrlForNodeName(firstLiveNode);
solrUrl = normalizeSolrUrl(solrUrl, false);
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions solr/core/src/test/org/apache/solr/cli/SolrCLITest.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ public void testResolveSolrUrl() {
assertEquals(SolrCLI.normalizeSolrUrl("http://localhost:8983/solr/"), "http://localhost:8983");
assertEquals(SolrCLI.normalizeSolrUrl("http://localhost:8983/"), "http://localhost:8983");
assertEquals(SolrCLI.normalizeSolrUrl("http://localhost:8983"), "http://localhost:8983");
assertEquals(
SolrCLI.normalizeSolrUrl("http://localhost:8983/solr/", false), "http://localhost:8983");
}

@Test
Expand Down
Loading