You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/11/01 19:09:03 UTC
[GitHub] [solr] risdenk commented on a diff in pull request #1158: SOLR-16368: experiment with builder to simplify client creation
risdenk commented on code in PR #1158:
URL: https://github.com/apache/solr/pull/1158#discussion_r1010801052
##########
solr/solrj/src/test/org/apache/solr/client/solrj/SolrSchemalessExampleTest.java:
##########
@@ -130,20 +130,18 @@ public void testFieldMutating() throws Exception {
@Override
public SolrClient createNewSolrClient() {
- try {
- // setup the server...
- String url = jetty.getBaseUrl().toString() + "/collection1";
- HttpSolrClient client = getHttpSolrClient(url, DEFAULT_CONNECTION_TIMEOUT);
- client.setUseMultiPartPost(random().nextBoolean());
-
- if (random().nextBoolean()) {
- client.setParser(new BinaryResponseParser());
- client.setRequestWriter(new BinaryRequestWriter());
- }
-
- return client;
- } catch (Exception ex) {
- throw new RuntimeException(ex);
+ // setup the server...
+ String url = jetty.getBaseUrl().toString() + "/collection1";
+ HttpSolrClient.Builder httpSolrClientBuilder = new HttpSolrClient.Builder(url);
+ if (random().nextBoolean()) {
+ httpSolrClientBuilder
+ .withRequestWriter(new BinaryRequestWriter())
+ .withResponseParser(new BinaryResponseParser());
+ }
+ if (random().nextBoolean()) {
+ httpSolrClientBuilder.withUseMultiPartPost(true);
}
Review Comment:
don't need to the if statement
```suggestion
httpSolrClientBuilder.withUseMultiPartPost(random().nextBoolean());
```
##########
solr/solrj/src/test/org/apache/solr/client/solrj/SolrSchemalessExampleTest.java:
##########
@@ -130,20 +130,18 @@ public void testFieldMutating() throws Exception {
@Override
public SolrClient createNewSolrClient() {
- try {
- // setup the server...
- String url = jetty.getBaseUrl().toString() + "/collection1";
- HttpSolrClient client = getHttpSolrClient(url, DEFAULT_CONNECTION_TIMEOUT);
- client.setUseMultiPartPost(random().nextBoolean());
-
- if (random().nextBoolean()) {
- client.setParser(new BinaryResponseParser());
- client.setRequestWriter(new BinaryRequestWriter());
- }
-
- return client;
- } catch (Exception ex) {
- throw new RuntimeException(ex);
+ // setup the server...
+ String url = jetty.getBaseUrl().toString() + "/collection1";
Review Comment:
should be `getServerUrl()`?
##########
solr/solrj/src/test/org/apache/solr/client/solrj/embedded/SolrExampleStreamingBinaryTest.java:
##########
@@ -35,9 +34,15 @@ public class SolrExampleStreamingBinaryTest extends SolrExampleStreamingTest {
@Override
public SolrClient createNewSolrClient() {
- ConcurrentUpdateSolrClient client = (ConcurrentUpdateSolrClient) super.createNewSolrClient();
- client.setParser(new BinaryResponseParser());
- client.setRequestWriter(new BinaryRequestWriter());
+
+ SolrClient client =
+ new ErrorTrackingConcurrentUpdateSolrClient.Builder(getServerUrl())
+ .withQueueSize(2)
+ .withThreadCount(5)
Review Comment:
not needed
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientBuilder.java:
##########
@@ -46,6 +49,16 @@ public B withResponseParser(ResponseParser responseParser) {
return getThis();
}
+ public B withRequestWriter(RequestWriter requestWriter) {
+ this.requestWriter = requestWriter;
+ return getThis();
+ }
+
+ public B withUseMultiPartPost(Boolean useMultiPartPost) {
Review Comment:
should deprecate `setUseMultiPartPost`?
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientBuilder.java:
##########
@@ -46,6 +49,16 @@ public B withResponseParser(ResponseParser responseParser) {
return getThis();
}
+ public B withRequestWriter(RequestWriter requestWriter) {
Review Comment:
should deprecate `setRequestWriter`?
##########
solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleBinaryTest.java:
##########
@@ -32,19 +32,15 @@ public static void beforeTest() throws Exception {
@Override
public SolrClient createNewSolrClient() {
- try {
- // setup the server...
- String url = jetty.getBaseUrl().toString() + "/collection1";
- HttpSolrClient client = getHttpSolrClient(url, DEFAULT_CONNECTION_TIMEOUT);
- client.setUseMultiPartPost(random().nextBoolean());
+ // setup the server...
+ String url = jetty.getBaseUrl().toString() + "/collection1";
Review Comment:
should be `getServerUrl()`?
##########
solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleXMLTest.java:
##########
@@ -32,15 +32,14 @@ public static void beforeTest() throws Exception {
@Override
public SolrClient createNewSolrClient() {
- try {
- String url = jetty.getBaseUrl().toString() + "/collection1";
- HttpSolrClient client = getHttpSolrClient(url, DEFAULT_CONNECTION_TIMEOUT);
- client.setUseMultiPartPost(random().nextBoolean());
- client.setParser(new XMLResponseParser());
- client.setRequestWriter(new RequestWriter());
- return client;
- } catch (Exception ex) {
- throw new RuntimeException(ex);
- }
+
+ String url = jetty.getBaseUrl().toString() + "/collection1";
Review Comment:
should be `getServerUrl()`
--
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