Re: [PR] Fix assert tool url normalization [solr]

2024-10-21 Thread via GitHub
epugh commented on PR #2778: URL: https://github.com/apache/solr/pull/2778#issuecomment-2427166378 Of the two URI builders, I think we would want the jakarta since at some point Apache HTTP Client goes away. What about just looking for `//` and removing one of them? In the 9x line

Re: [PR] Fix assert tool url normalization [solr]

2024-10-21 Thread via GitHub
malliaridis commented on PR #2778: URL: https://github.com/apache/solr/pull/2778#issuecomment-2426569848 @janhoy For some reason I am not surprised. I guess a `hostContext` that contains only `/` may be valid from the user's perspective, but we cannot handle it without additional workaround

Re: [PR] Fix assert tool url normalization [solr]

2024-10-21 Thread via GitHub
janhoy commented on PR #2778: URL: https://github.com/apache/solr/pull/2778#issuecomment-2426267830 @epugh, @malliaridis Turns out this still fails half the time, when `hostContext` is set to `/`. I can reproduce locally, about half the runs. See https://github.com/apache/solr/blob/b

Re: [PR] Fix assert tool url normalization [solr]

2024-10-20 Thread via GitHub
janhoy merged PR #2778: URL: https://github.com/apache/solr/pull/2778 -- 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

Re: [PR] Fix assert tool url normalization [solr]

2024-10-19 Thread via GitHub
janhoy commented on code in PR #2778: URL: https://github.com/apache/solr/pull/2778#discussion_r1807545831 ## solr/core/src/java/org/apache/solr/cli/PostTool.java: ## @@ -315,8 +320,17 @@ public void runImpl(CommandLine cli) throws Exception { throw new IllegalArgumentE

Re: [PR] Fix assert tool url normalization [solr]

2024-10-18 Thread via GitHub
epugh commented on PR #2778: URL: https://github.com/apache/solr/pull/2778#issuecomment-2423425973 Okay, so now the tests all passed @janhoy.. I kind of hacked in something to work for PostTool, I actually thought it would be needed on more other tools... -- This is an automated messag

Re: [PR] Fix assert tool url normalization [solr]

2024-10-18 Thread via GitHub
epugh commented on code in PR #2778: URL: https://github.com/apache/solr/pull/2778#discussion_r1807114387 ## solr/core/src/java/org/apache/solr/cli/AssertTool.java: ## @@ -531,7 +530,7 @@ private static boolean isSolrRunningOn(String url) throws Exception { } private st

Re: [PR] Fix assert tool url normalization [solr]

2024-10-18 Thread via GitHub
epugh commented on code in PR #2778: URL: https://github.com/apache/solr/pull/2778#discussion_r1807115198 ## solr/core/src/java/org/apache/solr/cli/PostTool.java: ## @@ -315,8 +315,7 @@ public void runImpl(CommandLine cli) throws Exception { throw new IllegalArgumentExc

Re: [PR] Fix assert tool url normalization [solr]

2024-10-18 Thread via GitHub
malliaridis commented on PR #2778: URL: https://github.com/apache/solr/pull/2778#issuecomment-2422560595 > The whole Solr Base Path and /solr suffix is a mess, and it is different in main than in branch_9x. So this will keep cropping up for as long as we backport stuff from 10 to 9 :(

Re: [PR] Fix assert tool url normalization [solr]

2024-10-18 Thread via GitHub
janhoy commented on code in PR #2778: URL: https://github.com/apache/solr/pull/2778#discussion_r1806335724 ## solr/core/src/java/org/apache/solr/cli/PostTool.java: ## @@ -315,8 +315,7 @@ public void runImpl(CommandLine cli) throws Exception { throw new IllegalArgumentEx

Re: [PR] Fix assert tool url normalization [solr]

2024-10-18 Thread via GitHub
epugh commented on code in PR #2778: URL: https://github.com/apache/solr/pull/2778#discussion_r1806318932 ## solr/core/src/java/org/apache/solr/cli/PostTool.java: ## @@ -315,8 +315,7 @@ public void runImpl(CommandLine cli) throws Exception { throw new IllegalArgumentExc

Re: [PR] Fix assert tool url normalization [solr]

2024-10-18 Thread via GitHub
epugh commented on PR #2778: URL: https://github.com/apache/solr/pull/2778#issuecomment-2422205861 @janhoy thanks for taking a stab at this... I definitly have been playing "whack a mole" of adding and removing /solr/'s in various places. I will go through this today I appreciate yo

Re: [PR] Fix assert tool url normalization [solr]

2024-10-18 Thread via GitHub
janhoy commented on code in PR #2778: URL: https://github.com/apache/solr/pull/2778#discussion_r1806175920 ## solr/core/src/java/org/apache/solr/cli/PostTool.java: ## @@ -315,8 +315,7 @@ public void runImpl(CommandLine cli) throws Exception { throw new IllegalArgumentEx

Re: [PR] Fix assert tool url normalization [solr]

2024-10-18 Thread via GitHub
janhoy commented on code in PR #2778: URL: https://github.com/apache/solr/pull/2778#discussion_r1806099337 ## solr/core/src/java/org/apache/solr/cli/SolrCLI.java: ## @@ -792,11 +792,10 @@ public static String normalizeSolrUrl(CommandLine cli) throws Exception { Str

Re: [PR] Fix assert tool url normalization [solr]

2024-10-18 Thread via GitHub
janhoy commented on code in PR #2778: URL: https://github.com/apache/solr/pull/2778#discussion_r1806091901 ## solr/core/src/java/org/apache/solr/cli/AssertTool.java: ## @@ -531,7 +530,7 @@ private static boolean isSolrRunningOn(String url) throws Exception { } private s

[PR] Fix assert tool url normalization [solr]

2024-10-18 Thread via GitHub
janhoy opened a new pull request, #2778: URL: https://github.com/apache/solr/pull/2778 Draft PR to get a green state in branch_9x Tests used to pass on branch_9x a few days ago. Some of the many `SolrCLI` changes touched the urlNormalization stuff so that you needed to add `/solr` at