You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "dsmiley (via GitHub)" <gi...@apache.org> on 2025/11/02 21:34:37 UTC
[PR] SOLR-17968: HttpSolrClient as new base class [solr]
dsmiley opened a new pull request, #3829:
URL: https://github.com/apache/solr/pull/3829
https://issues.apache.org/jira/browse/SOLR-17968
Review the commits separately. I'm likely to do a merge commit in the end.
Renamed {{org.apache.solr.client.solrj.apache.HttpSolrClient}} to {{HttpApacheSolrClient}}. Note that HttpSolrClient *used* to be in {{org.apache.solr.client.solrj.impl}}. Added {{org.apache.solr.client.solrj.HttpSolrClient}} as a pretty simple base class, which the obvious subclasses now as subclasses. Notice its in the base SolrJ package (a package that we should carefully choose what goes there; shouldn't have many things).
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17968: HttpSolrClient as new base class [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #3829:
URL: https://github.com/apache/solr/pull/3829#discussion_r2696328569
##########
solr/core/src/test/org/apache/solr/cloud/BasicDistributedZkTest.java:
##########
@@ -699,7 +699,7 @@ public void test() throws Exception {
for (SolrClient client : clients) {
assertEquals(
"unexpected pre-commitWithin document count on node: "
- + ((HttpSolrClient) client).getBaseURL(),
+ + ((HttpApacheSolrClient) client).getBaseURL(),
Review Comment:
These tests continue to use the legacy client, despite its name changing. There's a lot of work in updating the tests... I started it and shelved it for higher priorities: https://github.com/dsmiley/solr/pull/7 Pugh told me he's interested in taking over
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17968: HttpSolrClient as new base class [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #3829:
URL: https://github.com/apache/solr/pull/3829#issuecomment-3977240449
> This looks fine overall, but I'm still a little struck by the lack of docs. (Javadocs look great - referring more to higher-level docs like changelog entries, Upgrade Notes, etc.)
Yes of course; I was deferring that until closer to being done.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17968: HttpSolrClient as new base class [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #3829:
URL: https://github.com/apache/solr/pull/3829#issuecomment-3668566683
If I were to instead leave `org.apache.solr.client.solrj.apache.HttpSolrClient` with its current name in its own package, then the removal journey will mostly be changes to import statements without double-touching lines from HttpSolrClient -> HttpApacheSolrClient -> HttpSolrClient. Granted the interim period will be confusing in tests to know what you're looking at.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17968: HttpSolrClient as new base class [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #3829:
URL: https://github.com/apache/solr/pull/3829#discussion_r2867514928
##########
solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesDistrib.java:
##########
@@ -953,21 +953,22 @@ private void outOfOrderUpdatesIndividualReplicaTest() throws Exception {
// assert both replicas have same effect
for (SolrClient client : NONLEADERS) { // 0th is re-ordered replica, 1st is well-ordered replica
if (log.isInfoEnabled()) {
- log.info("Testing client: {}", ((HttpSolrClient) client).getBaseURL());
+ log.info("Testing client: {}", ((HttpApacheSolrClient) client).getBaseURL());
}
assertReplicaValue(
client,
0,
"inplace_updatable_float",
(newinplace_updatable_float + (float) (updates.size() - 1)),
"inplace_updatable_float didn't match for replica at client: "
- + ((HttpSolrClient) client).getBaseURL());
+ + ((HttpApacheSolrClient) client).getBaseURL());
Review Comment:
I'll address this by having HttpApacheSolrClient extend that. I forget what troubles I had but I'll figure it out.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17968: HttpSolrClient as new base class [solr]
Posted by "igiguere (via GitHub)" <gi...@apache.org>.
igiguere commented on code in PR #3829:
URL: https://github.com/apache/solr/pull/3829#discussion_r2696532600
##########
solr/core/src/test/org/apache/solr/cloud/BasicDistributedZkTest.java:
##########
@@ -699,7 +699,7 @@ public void test() throws Exception {
for (SolrClient client : clients) {
assertEquals(
"unexpected pre-commitWithin document count on node: "
- + ((HttpSolrClient) client).getBaseURL(),
+ + ((HttpApacheSolrClient) client).getBaseURL(),
Review Comment:
Good for me, then.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17968: HttpSolrClient as new base class [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #3829:
URL: https://github.com/apache/solr/pull/3829#issuecomment-3480851186
Come to think of it; I could split this between introducing `org.apache.solr.client.solrj.HttpSolrClient` (just touching a few files), and separately the rename of apache.HttpSolrClient to apache.HttpApacheSolrClient, which will include changing a number of import statements about where HttpSolrClient lives (touches many files). No merge commit; just 2 separate commits that incrementally progress things.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17968: HttpSolrClient as new base class [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #3829:
URL: https://github.com/apache/solr/pull/3829#discussion_r2867516684
##########
solr/solrj-jetty/src/java/org/apache/solr/client/solrj/jetty/HttpJettySolrClient.java:
##########
@@ -94,12 +93,8 @@
/**
* An HTTP {@link SolrClient} using Jetty {@link HttpClient}. This is Solr's most mature client for
* direct HTTP.
- *
- * <p>Despite the name, this client supports HTTP 1.1 and 2 -- toggle with {@link
- * HttpSolrClientBuilderBase#useHttp1_1(boolean)}. In retrospect, the name should have been {@code
Review Comment:
RE "should've been": good catch.
I'll ensure the docs say it supports HTTP 1.1 and 2.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17968: HttpSolrClient as new base class [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #3829:
URL: https://github.com/apache/solr/pull/3829#discussion_r2867513983
##########
changelog/unreleased/SOLR-17968-HttpSolrClient.yml:
##########
@@ -0,0 +1,8 @@
+
+title: Introduce org.apache.solr.client.solrj.HttpSolrClient, implemented by the Jetty & JDK HttpClient based SolrClients.
Review Comment:
Only "title" :-(
But we can write as much as we want in it.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17968: HttpSolrClient as new base class [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #3829:
URL: https://github.com/apache/solr/pull/3829#discussion_r3228441801
##########
solr/core/src/test/org/apache/solr/cloud/BasicDistributedZkTest.java:
##########
@@ -1150,7 +1150,7 @@ protected CollectionAdminResponse createCollection(
CollectionAdminResponse res = new CollectionAdminResponse();
if (client == null) {
- final String baseUrl = ((HttpSolrClient) clients.get(clientIndex)).getBaseURL();
+ final String baseUrl = ((HttpApacheSolrClient) clients.get(clientIndex)).getBaseURL();
Review Comment:
I'm about to push a revert of that rename in this PR (HttpApacheSolrClient goes back to HttpSolrClient). Thus 2 such classes will exist in separate packages. I have separate work that removes use of apache HttpSolrClient completely, and that which is nearly ready to share but I want this one in first.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17968: HttpSolrClient as new base class [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #3829:
URL: https://github.com/apache/solr/pull/3829#issuecomment-3693687954
I did a series of things, as seen in a series of commits:
1. HttpApacheSolrClient no longer extends HttpSolrClient in this PR. Unfortunate but it'll interfere...
2. Rename HttpSolrClientBase to HttpSolrClient. (replacing my earlier simply attempt at an abstract HttpSolrClient in this PR)
3. Rename HttpSolrClientBuilderBase.java to HttpSolrClient.BuilderBase (inner class, now)
4. Renamed HttpSolrClient.builder() internal instance method to `toBuilder(url)` to not conflict with...
5. Added HttpSolrClient.builder(url) static method, mostly using code moved from CloudSolrClient, which now uses this.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17968: HttpSolrClient as new base class [solr]
Posted by "igiguere (via GitHub)" <gi...@apache.org>.
igiguere commented on code in PR #3829:
URL: https://github.com/apache/solr/pull/3829#discussion_r2698740521
##########
solr/core/src/test/org/apache/solr/cloud/BasicDistributedZkTest.java:
##########
@@ -699,7 +699,7 @@ public void test() throws Exception {
for (SolrClient client : clients) {
assertEquals(
"unexpected pre-commitWithin document count on node: "
- + ((HttpSolrClient) client).getBaseURL(),
+ + ((HttpApacheSolrClient) client).getBaseURL(),
Review Comment:
@epugh : I'm on https://issues.apache.org/jira/browse/SOLR-17436 for now. Maybe later...
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17968: HttpSolrClient as new base class [solr]
Posted by "gerlowskija (via GitHub)" <gi...@apache.org>.
gerlowskija commented on PR #3829:
URL: https://github.com/apache/solr/pull/3829#issuecomment-3498070999
FWIW - I raised a few questions about the general naming of these clients on [this mailing list thread](https://lists.apache.org/thread/84j5kc61n1ndt2ors1ozqw6dx93og30k). I don't want that discussion to hold up this PR, but adding the link here as it is relevant if folks end up liking that idea...
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17968: HttpSolrClient as new base class [solr]
Posted by "igiguere (via GitHub)" <gi...@apache.org>.
igiguere commented on code in PR #3829:
URL: https://github.com/apache/solr/pull/3829#discussion_r2696164988
##########
solr/core/src/test/org/apache/solr/cloud/BasicDistributedZkTest.java:
##########
@@ -699,7 +699,7 @@ public void test() throws Exception {
for (SolrClient client : clients) {
assertEquals(
"unexpected pre-commitWithin document count on node: "
- + ((HttpSolrClient) client).getBaseURL(),
+ + ((HttpApacheSolrClient) client).getBaseURL(),
Review Comment:
Here and elsewhere:
HttpApacheSolrClient is supposed to die, but all these tests now use it instead of HttpSolrClient: the new one should be used.
IMHO, this looks weird, and it will all have to change eventually anyways when HttpApacheSolrClient is removed.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17968: HttpSolrClient as new base class [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #3829:
URL: https://github.com/apache/solr/pull/3829#discussion_r3228493348
##########
solr/test-framework/src/java/org/apache/solr/client/solrj/apache/package-info.java:
##########
@@ -16,4 +16,5 @@
*/
/** Apache HttpClient based {@link org.apache.solr.client.solrj.SolrClient} implementations */
+@Deprecated
Review Comment:
CHANGELOG.md 10.0:
> Solr no longer uses Apache HttpClient. Nonetheless SolrJ still includes deprecated SolrClient implementations for that (which will be removed eventually), and many tests still use it. Also, there were server metrics relating to HTTP client connection pools that no longer exist. [SOLR-17872](https://issues.apache.org/jira/browse/SOLR-17872) (David Smiley)
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17968: HttpSolrClient as new base class [solr]
Posted by "gerlowskija (via GitHub)" <gi...@apache.org>.
gerlowskija commented on PR #3829:
URL: https://github.com/apache/solr/pull/3829#issuecomment-4670808510
Missed this a few weeks back, but really glad to see it got in. Great stuff David!
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17968: HttpSolrClient as new base class [solr]
Posted by "gerlowskija (via GitHub)" <gi...@apache.org>.
gerlowskija commented on code in PR #3829:
URL: https://github.com/apache/solr/pull/3829#discussion_r2499543142
##########
solr/test-framework/src/java/org/apache/solr/client/solrj/apache/SolrPortAwareCookieSpecFactory.java:
##########
@@ -31,6 +31,7 @@
import org.apache.http.protocol.HttpContext;
@SuppressWarnings("deprecation")
+@Deprecated
Review Comment:
[Q] Why is this deprecated in this PR? Is it just a part of the larger "apache client" deprecation that was missed until now?
##########
solr/solrj/src/test/org/apache/solr/client/solrj/impl/BasicHttpSolrClientTest.java:
##########
@@ -548,7 +548,7 @@ public void testUpdate() throws Exception {
assertEquals("post", DebugServlet.lastMethod);
assertEquals(
- "Solr[" + HttpSolrClient.class.getName() + "] " + UA_VERSION,
+ "Solr[" + HttpApacheSolrClient.class.getName() + "] " + UA_VERSION,
Review Comment:
[0] Holy duplication. I understand that we shouldn't delve into refactors or unrelated cleanups in this PR, but what a shame this test doesn't use a constant or something here.
##########
solr/solrj/src/test/org/apache/solr/client/solrj/impl/BasicHttpSolrClientTest.java:
##########
Review Comment:
[Q] Should this test class itself be renamed, as it seems like it's specific to the "Apache" version of the client?
##########
solr/solrj/src/java/org/apache/solr/client/solrj/HttpSolrClient.java:
##########
@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.solrj;
+
+/** A {@link SolrClient} based on HTTP. */
Review Comment:
[Q] I can imagine folks familiar with the old concrete implementation "HttpSolrClient" coming here, finding it's now abstract, and not being sure which implementation they should actually use.
Wdyt about adding a few `@see` tags here pointing at the different implementations, or alternatively a sentence or two to the effect of: "If you're looking for a single-url client to actually use, go check out..."
##########
changelog/unreleased/SOLR-17968-HttpSolrClient.yml:
##########
@@ -0,0 +1,8 @@
+
+title: Introduce org.apache.solr.client.solrj.HttpSolrClient, implemented by the Jetty & JDK HttpClient based SolrClients.
Review Comment:
[Q] Is it worth mentioning the Apache SolrClient here as well? I know it's deprecated, but the conspicuous absence might imply to folks that HttpApacheSolrClient *doesn't* inherit from HttpSolrClient.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17968: HttpSolrClient as new base class [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #3829:
URL: https://github.com/apache/solr/pull/3829#discussion_r2500454379
##########
solr/solrj/src/test/org/apache/solr/client/solrj/impl/BasicHttpSolrClientTest.java:
##########
Review Comment:
good observation; yes I think this test is specific to Apache HttpClient, so it should be renamed (or not), moved, and eventually removed. We have other tests naming the other clients and their names; ideally this one would have been renamed when those came into being.
##########
solr/test-framework/src/java/org/apache/solr/client/solrj/apache/SolrPortAwareCookieSpecFactory.java:
##########
@@ -31,6 +31,7 @@
import org.apache.http.protocol.HttpContext;
@SuppressWarnings("deprecation")
+@Deprecated
Review Comment:
Yes; notice the package name.
This really underscores to me why our different implementations ought to get their very own packages!
##########
changelog/unreleased/SOLR-17968-HttpSolrClient.yml:
##########
@@ -0,0 +1,8 @@
+
+title: Introduce org.apache.solr.client.solrj.HttpSolrClient, implemented by the Jetty & JDK HttpClient based SolrClients.
Review Comment:
HttpApacheSolrClient has a death sentence; I don't want to speak of it more than necessary.
##########
solr/solrj/src/java/org/apache/solr/client/solrj/HttpSolrClient.java:
##########
@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.solrj;
+
+/** A {@link SolrClient} based on HTTP. */
Review Comment:
great idea
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17968: HttpSolrClient as new base class [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #3829:
URL: https://github.com/apache/solr/pull/3829#discussion_r3229410703
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java:
##########
@@ -21,8 +21,8 @@
import org.apache.solr.common.util.IOUtils;
/**
- * This {@link CloudSolrClient} is a base implementation using a {@link HttpSolrClientBase}. The '2'
- * in the name has no differentiating significance anymore.
+ * This {@link CloudSolrClient} is a base implementation using a {@link HttpSolrClient}. The '2' in
Review Comment:
the 2 refers to CloudHttp2SolrClient
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17968: HttpSolrClient as new base class [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #3829:
URL: https://github.com/apache/solr/pull/3829#discussion_r3229430958
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java:
##########
@@ -104,7 +97,24 @@ protected HttpSolrClientBase(String serverBaseUrl, HttpSolrClientBuilderBase<?,
}
}
- public abstract HttpSolrClientBuilderBase<?, ?> builder();
+ private static String extractBaseUrl(String serverBaseUrl) {
+ if (serverBaseUrl == null) {
+ return null;
+ }
+ if (!serverBaseUrl.equals("/") && serverBaseUrl.endsWith("/")) {
+ serverBaseUrl = serverBaseUrl.substring(0, serverBaseUrl.length() - 1);
+ }
+
+ if (serverBaseUrl.startsWith("//")) {
+ serverBaseUrl = serverBaseUrl.substring(1);
+ }
+ return serverBaseUrl;
+ }
+
+ /** Typically looks like {@code http://localhost:8983/solr} (no core or collection) */
+ public String getBaseURL() {
Review Comment:
I kinda wish we use URL as a type instead of a String both for added type safety and also likely avoids extra parsing. Jetty & JDK HttpClients natively works with a URI -- passing a String results in URI parsing _anyway_.
I suppose we could add getBaseUri at some point.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17968: HttpSolrClient as new base class [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley merged PR #3829:
URL: https://github.com/apache/solr/pull/3829
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17968: HttpSolrClient as new base class [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #3829:
URL: https://github.com/apache/solr/pull/3829#discussion_r2696550220
##########
solr/core/src/test/org/apache/solr/cloud/BasicDistributedZkTest.java:
##########
@@ -699,7 +699,7 @@ public void test() throws Exception {
for (SolrClient client : clients) {
assertEquals(
"unexpected pre-commitWithin document count on node: "
- + ((HttpSolrClient) client).getBaseURL(),
+ + ((HttpApacheSolrClient) client).getBaseURL(),
Review Comment:
I would be happy to work with you @igiguere on this if you want. I do not have any special knowledge if you want to pick it up!
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org