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/09/14 18:37:11 UTC

[GitHub] [solr] epugh opened a new pull request, #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

epugh opened a new pull request, #1012:
URL: https://github.com/apache/solr/pull/1012

   https://issues.apache.org/jira/browse/SOLR-16368
   
   # Description
   
   Where can we use `SolrClient` or `SolrCloudClient` instead of the sub classes?
   
   # Solution
   
   hunting through source!
   
   # Tests
   
   Make the change, run the tests...
   
   # Checklist
   
   Please review the following and check all that apply:
   
   - [ ] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
   - [ ] I have created a Jira issue and added the issue ID to my pull request title.
   - [ ] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
   - [ ] I have developed this patch against the `main` branch.
   - [ ] I have run `./gradlew check`.
   - [ ] I have added tests for my changes.
   - [ ] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide)
   


-- 
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

[GitHub] [solr] dsmiley commented on pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
dsmiley commented on PR #1012:
URL: https://github.com/apache/solr/pull/1012#issuecomment-1247265686

   > Thoughts on introducing a getHttp1Client method in the base test class that would take a SolrClient and could interrogate it and return the httpClient? That would remove a ton of places where we refer to CloudLegacySolrClient.
   
   I spot checked some tests to see why Apache HttpClient is used.
   * In some tests, the test just needs any Http client really.  These can be migrated to Java's new [built-in Http client](https://www.baeldung.com/java-9-http-client).
   * In some tests, another SolrClient is built using the same backing Apache HttpClient.  I suspect this is merely a convenience.  If there was a SolrClient.clone() method, say, then this could work?
   * In some tests, the objective is merely to change the Collection being targeted.  Maybe a SolrClient.forCollection(String) returning SolrClient?  Just an idea.
   
   Despite these ideas, implementing them is out of scope of this issue, I think, even though they would influence each other.  They could be tackled first or after.


-- 
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

[GitHub] [solr] epugh commented on a diff in pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
epugh commented on code in PR #1012:
URL: https://github.com/apache/solr/pull/1012#discussion_r992644882


##########
solr/core/src/test/org/apache/solr/TestDistributedSearch.java:
##########
@@ -1711,7 +1710,7 @@ public void test() throws Exception {
           tdate_b,
           "stats.calcdistinct",
           "true");
-    } catch (BaseHttpSolrClient.RemoteSolrException e) {
+    } catch (SolrException e) {

Review Comment:
   Humm....    I think maybe to just eliminate the imnport?   I'm going to back it out however, as maybe we don't want to catch such as a general SolrException...



-- 
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

[GitHub] [solr] epugh commented on pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
epugh commented on PR #1012:
URL: https://github.com/apache/solr/pull/1012#issuecomment-1247936897

   THanks for sharing those thoughts @dsmiley .....   I'm going to keep plugging away, and try and stay away from refactoring things in this 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: 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

[GitHub] [solr] epugh commented on pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
epugh commented on PR #1012:
URL: https://github.com/apache/solr/pull/1012#issuecomment-1276146141

   I went ahead and looked at the other specific subclasses of `SolrClient` and `SolrCloudClient`...    I think at THIS POINT, I am now done with this PR.  I am running all the tests again, and assuming they pass and a I get LGTM, then I'll merge.   I will start a thread on Dev about other refactoring/cleanup areas, maybe `newdev` candidates?


-- 
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

[GitHub] [solr] epugh commented on pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
epugh commented on PR #1012:
URL: https://github.com/apache/solr/pull/1012#issuecomment-1274436928

   @dsmiley I think I am done!   Only had to touch 105 files ;-).   I have a list of improvements I'd like to make, but those can be for follow up PR's...


-- 
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

[GitHub] [solr] dsmiley commented on a diff in pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
dsmiley commented on code in PR #1012:
URL: https://github.com/apache/solr/pull/1012#discussion_r991444078


##########
solr/core/src/test/org/apache/solr/filestore/TestDistribPackageStore.java:
##########
@@ -192,10 +192,10 @@ public static void checkAllNodesForFile(
       assertResponseValues(10, new Fetcher(url, jettySolrRunner), expected);
 
       if (verifyContent) {
-        try (HttpSolrClient solrClient = (HttpSolrClient) jettySolrRunner.newClient()) {
+        try (HttpSolrClient httpSolrClient = (HttpSolrClient) jettySolrRunner.newClient()) {

Review Comment:
   Sure but don't bother doing renames for variables already declared to be of type SolrClient, as that would increase the scope massively.  Basically, while you're loosening a type declaration, take this opportunity to consider making the variable name more general as well.  That is a plenty wide scope (in terms of # of files) as it is.



-- 
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

[GitHub] [solr] dsmiley commented on a diff in pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
dsmiley commented on code in PR #1012:
URL: https://github.com/apache/solr/pull/1012#discussion_r992564452


##########
solr/core/src/test/org/apache/solr/filestore/TestDistribPackageStore.java:
##########
@@ -309,11 +309,10 @@ public static <T extends NavigableObject> T assertResponseValues(
   public static void uploadKey(byte[] bytes, String path, MiniSolrCloudCluster cluster)
       throws Exception {
     JettySolrRunner jetty = cluster.getRandomJetty(random());
-    try (HttpSolrClient client = (HttpSolrClient) jetty.newClient()) {
-      PackageUtils.uploadKey(
-          bytes, path, Paths.get(jetty.getCoreContainer().getSolrHome()), client);
+    try (HttpSolrClient solrClient = (HttpSolrClient) jetty.newClient()) {
+      PackageUtils.uploadKey(bytes, path, Paths.get(jetty.getCoreContainer().getSolrHome()));
       String url = jetty.getBaseURLV2() + "/node/files" + path + "?sync=true";
-      Object resp = Utils.executeGET(client.getHttpClient(), url, null);
+      Object resp = Utils.executeGET(solrClient.getHttpClient(), url, null);
       log.info("sync resp: {} was {}", url, resp);
     }

Review Comment:
   This code doesn't need an HttpSolrClient at all; it only needs some HTTP client.  Maybe we'll punt on this for now but hopefully we'll come back.  At least a comment for now?



##########
solr/core/src/test/org/apache/solr/handler/admin/ZookeeperStatusHandlerFailureTest.java:
##########
@@ -51,12 +51,13 @@ public void monitorZookeeperAfterZkShutdown()
       throws IOException, SolrServerException, InterruptedException, ExecutionException,
           TimeoutException {
     URL baseUrl = cluster.getJettySolrRunner(0).getBaseUrl();
-    HttpSolrClient solr = new HttpSolrClient.Builder(baseUrl.toString()).build();
+    HttpSolrClient solrClient = new HttpSolrClient.Builder(baseUrl.toString()).build();

Review Comment:
   Why change this file at all if you didn't change the declared type?



##########
solr/core/src/test/org/apache/solr/handler/admin/ZookeeperReadAPITest.java:
##########
@@ -60,19 +60,20 @@ public void testZkread() throws Exception {
     String basezk = baseUrl.toString().replace("/solr", "/api") + "/cluster/zk/data";
     String basezkls = baseUrl.toString().replace("/solr", "/api") + "/cluster/zk/ls";
 
-    try (HttpSolrClient client = new HttpSolrClient.Builder(baseUrl.toString()).build()) {
+    try (HttpSolrClient solrClient = new HttpSolrClient.Builder(baseUrl.toString()).build()) {

Review Comment:
   This SolrClient isn't actually used as-such.  Maybe a TODO to avoid this later.
   Or just reverse -- don't change this file; the declared type is fine.  I'd have preferred "var" but I failed to convince you.



##########
solr/core/src/test/org/apache/solr/security/BasicAuthStandaloneTest.java:
##########
@@ -88,33 +88,37 @@ public void testBasicAuth() throws Exception {
     String authcPrefix = "/admin/authentication";
     String authzPrefix = "/admin/authorization";
 
-    HttpClient cl = null;
-    HttpSolrClient httpSolrClient = null;
+    HttpClient httpClient = null;

Review Comment:
   nice renames to clarify!



##########
solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/TopicStream.java:
##########
@@ -509,7 +509,7 @@ private void getPersistedCheckpoints() throws IOException {
       for (Replica replica : replicas) {
         if (replica.getState() == Replica.State.ACTIVE
             && liveNodes.contains(replica.getNodeName())) {
-          HttpSolrClient httpClient =
+          SolrClient httpClient =

Review Comment:
   rename variable please!  misleading name



##########
solr/solrj/src/java/org/apache/solr/client/solrj/io/SolrClientCache.java:
##########
@@ -100,10 +100,11 @@ public synchronized CloudSolrClient getCloudSolrClient(String zkHost) {
   }
 
   @Deprecated(since = "9.0")
-  public synchronized HttpSolrClient getHttpSolrClient(String host) {
-    HttpSolrClient client;
+  // is this really deprecated?  what if it's getSolrClient?  It COULD be!

Review Comment:
   The name is fine because the argument is in fact a URL.  But I agree it should not be deprecated.



##########
solr/core/src/test/org/apache/solr/handler/admin/ZookeeperStatusHandlerTest.java:
##########
@@ -76,12 +76,13 @@ public void monitorZookeeper()
       throws IOException, SolrServerException, InterruptedException, ExecutionException,
           TimeoutException {
     URL baseUrl = cluster.getJettySolrRunner(0).getBaseUrl();
-    HttpSolrClient solr = new HttpSolrClient.Builder(baseUrl.toString()).build();
+    HttpSolrClient solrClient = new HttpSolrClient.Builder(baseUrl.toString()).build();

Review Comment:
   Why change this file at all if you didn't change the declared type?



##########
solr/core/src/test/org/apache/solr/update/SolrCmdDistributorTest.java:
##########
@@ -376,9 +376,9 @@ public void newSearcher(
   }
 
   private void testDeletes(boolean dbq, boolean withFailures) throws Exception {
-    final HttpSolrClient solrclient = (HttpSolrClient) clients.get(0);
-    solrclient.commit(true, true);
-    long numFoundBefore = solrclient.query(new SolrQuery("*:*")).getResults().getNumFound();
+    final HttpSolrClient solrClient = (HttpSolrClient) clients.get(0);
+    solrClient.commit(true, true);
+    long numFoundBefore = solrClient.query(new SolrQuery("*:*")).getResults().getNumFound();

Review Comment:
   Why change this file at all if you didn't change the declared type?



##########
solr/core/src/test/org/apache/solr/TestDistributedSearch.java:
##########
@@ -1711,7 +1710,7 @@ public void test() throws Exception {
           tdate_b,
           "stats.calcdistinct",
           "true");
-    } catch (BaseHttpSolrClient.RemoteSolrException e) {
+    } catch (SolrException e) {

Review Comment:
   Why?



##########
solr/solrj/src/java/org/apache/solr/client/solrj/io/SolrClientCache.java:
##########
@@ -100,10 +100,11 @@ public synchronized CloudSolrClient getCloudSolrClient(String zkHost) {
   }
 
   @Deprecated(since = "9.0")
-  public synchronized HttpSolrClient getHttpSolrClient(String host) {
-    HttpSolrClient client;
+  // is this really deprecated?  what if it's getSolrClient?  It COULD be!

Review Comment:
   I'd appreciate docs on the argument if you have time.  It's possibly kind of wrong; appears to be baseUrl actually?



-- 
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

[GitHub] [solr] dsmiley commented on a diff in pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
dsmiley commented on code in PR #1012:
URL: https://github.com/apache/solr/pull/1012#discussion_r972096194


##########
solr/core/src/java/org/apache/solr/util/ExportTool.java:
##########
@@ -490,7 +491,7 @@ class CoreHandler {
       }
 
       boolean exportDocsFromCore() throws IOException, SolrServerException {
-        HttpSolrClient client = new HttpSolrClient.Builder(baseurl).build();
+        SolrClient client = new HttpSolrClient.Builder(baseurl).build();

Review Comment:
   Up to you, but if you can easily also do try-with-resources pattern as well, that'd be nice.



-- 
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

[GitHub] [solr] dsmiley commented on a diff in pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
dsmiley commented on code in PR #1012:
URL: https://github.com/apache/solr/pull/1012#discussion_r992533454


##########
solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java:
##########
@@ -2747,7 +2747,7 @@ protected void waitForAllWarmingSearchers() throws InterruptedException {
   }
 
   protected long getIndexVersion(Replica replica) throws IOException {
-    try (HttpSolrClient client = new HttpSolrClient.Builder(replica.getCoreUrl()).build()) {
+    try (SolrClient client = new HttpSolrClient.Builder(replica.getCoreUrl()).build()) {

Review Comment:
   @sonatype-lift ignoreall
   Because OBJECTS is ConcurrentHashMap so this is bogus.



-- 
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

[GitHub] [solr] dsmiley commented on a diff in pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
dsmiley commented on code in PR #1012:
URL: https://github.com/apache/solr/pull/1012#discussion_r991444078


##########
solr/core/src/test/org/apache/solr/filestore/TestDistribPackageStore.java:
##########
@@ -192,10 +192,10 @@ public static void checkAllNodesForFile(
       assertResponseValues(10, new Fetcher(url, jettySolrRunner), expected);
 
       if (verifyContent) {
-        try (HttpSolrClient solrClient = (HttpSolrClient) jettySolrRunner.newClient()) {
+        try (HttpSolrClient httpSolrClient = (HttpSolrClient) jettySolrRunner.newClient()) {

Review Comment:
   Sure but don't bother doing renames for variables already declared to be of type SolrClient, as that would increase the scope massively.



-- 
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

[GitHub] [solr] epugh commented on a diff in pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
epugh commented on code in PR #1012:
URL: https://github.com/apache/solr/pull/1012#discussion_r992714597


##########
solr/solrj/src/java/org/apache/solr/client/solrj/io/SolrClientCache.java:
##########
@@ -100,10 +100,11 @@ public synchronized CloudSolrClient getCloudSolrClient(String zkHost) {
   }
 
   @Deprecated(since = "9.0")
-  public synchronized HttpSolrClient getHttpSolrClient(String host) {
-    HttpSolrClient client;
+  // is this really deprecated?  what if it's getSolrClient?  It COULD be!

Review Comment:
   i think this is a good follow up candidate...   I updated the host --> baseUrl, but I think we also should undeprecate these methods...    in another issue?



-- 
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

[GitHub] [solr] dsmiley commented on a diff in pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
dsmiley commented on code in PR #1012:
URL: https://github.com/apache/solr/pull/1012#discussion_r990734533


##########
solr/core/src/test/org/apache/solr/cloud/DistributedVersionInfoTest.java:
##########
@@ -58,7 +56,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-@LuceneTestCase.Nightly // Lots of sleeps to introduce timing delays?
+// @LuceneTestCase.Nightly // Lots of sleeps to introduce timing delays?

Review Comment:
   Why?



-- 
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

[GitHub] [solr] sonatype-lift[bot] commented on a diff in pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on code in PR #1012:
URL: https://github.com/apache/solr/pull/1012#discussion_r990689438


##########
solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java:
##########
@@ -141,7 +141,7 @@ public void addKey(byte[] key, String destinationKeyFilename) throws Exception {
 
     // put the public key into package store's trusted key store and request a sync.
     String path = PackageStoreAPI.KEYS_DIR + "/" + destinationKeyFilename;
-    PackageUtils.uploadKey(key, path, Paths.get(solrHome), solrClient);
+    PackageUtils.uploadKey(key, path, Paths.get(solrHome));

Review Comment:
   *[PATH_TRAVERSAL_IN](https://find-sec-bugs.github.io/bugs.htm#PATH_TRAVERSAL_IN):*  This API (java/nio/file/Paths.get(Ljava/lang/String;[Ljava/lang/String;)Ljava/nio/file/Path;) reads a file whose location might be specified by user input
   
   ---
   
   <details><summary><b>ℹī¸ Learn about @sonatype-lift commands</b></summary>
   
   You can reply with the following commands. For example, reply with ***@sonatype-lift ignoreall*** to leave out all findings.
   | **Command** | **Usage** |
   | ------------- | ------------- |
   | `@sonatype-lift ignore` | Leave out the above finding from this PR |
   | `@sonatype-lift ignoreall` | Leave out all the existing findings from this PR |
   | `@sonatype-lift exclude <file\|issue\|path\|tool>` | Exclude specified `file\|issue\|path\|tool` from Lift findings by updating your config.toml file |
   
   **Note:** When talking to LiftBot, you need to **refresh** the page to see its response.
   <sub>[Click here](https://github.com/apps/sonatype-lift/installations/new) to add LiftBot to another repo.</sub></details>
   
   
   
   ---
   
   Was this a good recommendation?
   [ [🙁 Not relevant](https://www.sonatype.com/lift-comment-rating?comment=343591638&lift_comment_rating=1) ] - [ [😕 Won't fix](https://www.sonatype.com/lift-comment-rating?comment=343591638&lift_comment_rating=2) ] - [ [😑 Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=343591638&lift_comment_rating=3) ] - [ [🙂 Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=343591638&lift_comment_rating=4) ] - [ [😊 Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=343591638&lift_comment_rating=5) ]



-- 
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

[GitHub] [solr] dsmiley commented on a diff in pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
dsmiley commented on code in PR #1012:
URL: https://github.com/apache/solr/pull/1012#discussion_r990734673


##########
solr/core/src/test/org/apache/solr/filestore/TestDistribPackageStore.java:
##########
@@ -192,10 +192,10 @@ public static void checkAllNodesForFile(
       assertResponseValues(10, new Fetcher(url, jettySolrRunner), expected);
 
       if (verifyContent) {
-        try (HttpSolrClient solrClient = (HttpSolrClient) jettySolrRunner.newClient()) {
+        try (HttpSolrClient httpSolrClient = (HttpSolrClient) jettySolrRunner.newClient()) {

Review Comment:
   solrClient was a totally fine name.  But rename if you wish.



-- 
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

[GitHub] [solr] epugh commented on a diff in pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
epugh commented on code in PR #1012:
URL: https://github.com/apache/solr/pull/1012#discussion_r991228890


##########
solr/core/src/test/org/apache/solr/cloud/DistributedVersionInfoTest.java:
##########
@@ -58,7 +56,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-@LuceneTestCase.Nightly // Lots of sleeps to introduce timing delays?
+// @LuceneTestCase.Nightly // Lots of sleeps to introduce timing delays?

Review Comment:
   argh, this was so I could test this test...   I need to learn how to pass parameters to tests so I can run nightly!  good catch...



-- 
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

[GitHub] [solr] dsmiley commented on pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
dsmiley commented on PR #1012:
URL: https://github.com/apache/solr/pull/1012#issuecomment-1275097753

   BTW, there are *other* needlessly specific declarations of SolrClient subclasses as well.  I can see this PR focuses on HttpSolrClient.  Maybe you'll do others (like Http2SolrClient etc.) in other PRs?


-- 
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

[GitHub] [solr] dsmiley commented on a diff in pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
dsmiley commented on code in PR #1012:
URL: https://github.com/apache/solr/pull/1012#discussion_r992765842


##########
solr/solrj/src/java/org/apache/solr/client/solrj/io/SolrClientCache.java:
##########
@@ -100,10 +100,11 @@ public synchronized CloudSolrClient getCloudSolrClient(String zkHost) {
   }
 
   @Deprecated(since = "9.0")
-  public synchronized HttpSolrClient getHttpSolrClient(String host) {
-    HttpSolrClient client;
+  // is this really deprecated?  what if it's getSolrClient?  It COULD be!

Review Comment:
   in another issue is fine!



-- 
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

[GitHub] [solr] dsmiley commented on a diff in pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
dsmiley commented on code in PR #1012:
URL: https://github.com/apache/solr/pull/1012#discussion_r992642897


##########
solr/core/src/test/org/apache/solr/filestore/TestDistribPackageStore.java:
##########
@@ -309,11 +309,10 @@ public static <T extends NavigableObject> T assertResponseValues(
   public static void uploadKey(byte[] bytes, String path, MiniSolrCloudCluster cluster)
       throws Exception {
     JettySolrRunner jetty = cluster.getRandomJetty(random());
-    try (HttpSolrClient client = (HttpSolrClient) jetty.newClient()) {
-      PackageUtils.uploadKey(
-          bytes, path, Paths.get(jetty.getCoreContainer().getSolrHome()), client);
+    try (HttpSolrClient solrClient = (HttpSolrClient) jetty.newClient()) {
+      PackageUtils.uploadKey(bytes, path, Paths.get(jetty.getCoreContainer().getSolrHome()));
       String url = jetty.getBaseURLV2() + "/node/files" + path + "?sync=true";
-      Object resp = Utils.executeGET(client.getHttpClient(), url, null);
+      Object resp = Utils.executeGET(solrClient.getHttpClient(), url, null);
       log.info("sync resp: {} was {}", url, resp);
     }

Review Comment:
   Seems like an independent issue to do then.  So you could revert this file.



-- 
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

[GitHub] [solr] dsmiley commented on a diff in pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
dsmiley commented on code in PR #1012:
URL: https://github.com/apache/solr/pull/1012#discussion_r990733430


##########
solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java:
##########
@@ -141,7 +141,7 @@ public void addKey(byte[] key, String destinationKeyFilename) throws Exception {
 
     // put the public key into package store's trusted key store and request a sync.
     String path = PackageStoreAPI.KEYS_DIR + "/" + destinationKeyFilename;
-    PackageUtils.uploadKey(key, path, Paths.get(solrHome), solrClient);
+    PackageUtils.uploadKey(key, path, Paths.get(solrHome));

Review Comment:
   @sonatype-lift ignoreall
   (because we're just refactoring)



-- 
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

[GitHub] [solr] epugh commented on a diff in pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
epugh commented on code in PR #1012:
URL: https://github.com/apache/solr/pull/1012#discussion_r991255182


##########
solr/core/src/java/org/apache/solr/util/ExportTool.java:
##########
@@ -490,7 +491,7 @@ class CoreHandler {
       }
 
       boolean exportDocsFromCore() throws IOException, SolrServerException {
-        HttpSolrClient client = new HttpSolrClient.Builder(baseurl).build();
+        SolrClient client = new HttpSolrClient.Builder(baseurl).build();

Review Comment:
   @dsmiley I just updated this to what I think is try with resrouces...   Does this mean the `.close()` gets magically called?  how does it know to do that?
   



-- 
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

[GitHub] [solr] epugh commented on a diff in pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
epugh commented on code in PR #1012:
URL: https://github.com/apache/solr/pull/1012#discussion_r991255673


##########
solr/core/src/java/org/apache/solr/util/ExportTool.java:
##########
@@ -490,7 +491,7 @@ class CoreHandler {
       }
 
       boolean exportDocsFromCore() throws IOException, SolrServerException {
-        HttpSolrClient client = new HttpSolrClient.Builder(baseurl).build();
+        SolrClient client = new HttpSolrClient.Builder(baseurl).build();

Review Comment:
   never mind, read more detail in https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html



-- 
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

[GitHub] [solr] dsmiley commented on a diff in pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
dsmiley commented on code in PR #1012:
URL: https://github.com/apache/solr/pull/1012#discussion_r993854042


##########
solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/TopicStream.java:
##########
@@ -509,7 +509,7 @@ private void getPersistedCheckpoints() throws IOException {
       for (Replica replica : replicas) {
         if (replica.getState() == Replica.State.ACTIVE
             && liveNodes.contains(replica.getNodeName())) {
-          HttpSolrClient httpClient =
+          SolrClient httpClient =

Review Comment:
   pending still



-- 
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

[GitHub] [solr] epugh commented on a diff in pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
epugh commented on code in PR #1012:
URL: https://github.com/apache/solr/pull/1012#discussion_r992641891


##########
solr/core/src/test/org/apache/solr/filestore/TestDistribPackageStore.java:
##########
@@ -309,11 +309,10 @@ public static <T extends NavigableObject> T assertResponseValues(
   public static void uploadKey(byte[] bytes, String path, MiniSolrCloudCluster cluster)
       throws Exception {
     JettySolrRunner jetty = cluster.getRandomJetty(random());
-    try (HttpSolrClient client = (HttpSolrClient) jetty.newClient()) {
-      PackageUtils.uploadKey(
-          bytes, path, Paths.get(jetty.getCoreContainer().getSolrHome()), client);
+    try (HttpSolrClient solrClient = (HttpSolrClient) jetty.newClient()) {
+      PackageUtils.uploadKey(bytes, path, Paths.get(jetty.getCoreContainer().getSolrHome()));
       String url = jetty.getBaseURLV2() + "/node/files" + path + "?sync=true";
-      Object resp = Utils.executeGET(client.getHttpClient(), url, null);
+      Object resp = Utils.executeGET(solrClient.getHttpClient(), url, null);
       log.info("sync resp: {} was {}", url, resp);
     }

Review Comment:
   I can add a comment, however this pattern shows up in many places.   I would *really* like to see a separate Issue with a recommendation on where we can get a HttpClient....



-- 
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

[GitHub] [solr] sonatype-lift[bot] commented on a diff in pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on code in PR #1012:
URL: https://github.com/apache/solr/pull/1012#discussion_r992155605


##########
solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java:
##########
@@ -2747,7 +2747,7 @@ protected void waitForAllWarmingSearchers() throws InterruptedException {
   }
 
   protected long getIndexVersion(Replica replica) throws IOException {
-    try (HttpSolrClient client = new HttpSolrClient.Builder(replica.getCoreUrl()).build()) {
+    try (SolrClient client = new HttpSolrClient.Builder(replica.getCoreUrl()).build()) {

Review Comment:
   đŸ’Ŧ 5 similar findings have been found in this PR
   
   ---
   
   *THREAD_SAFETY_VIOLATION:*  Unprotected write. Non-private method `AbstractFullDistribZkTestBase.getIndexVersion(...)` indirectly mutates container `util.ObjectReleaseTracker.OBJECTS` via call to `Map.put(...)` outside of synchronization.
    Reporting because another access to the same memory occurs on a background thread, although this access may not.
   
   ---
   
   <details><summary><b>🔎 Expand here to view all instances of this finding</b></summary><br/>
   
   <div align="center">
   
   | **File Path** | **Line Number** |
   | ------------- | ------------- |
   | solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java | [2208](https://github.com/epugh/solr/blob/97a53cb37ab30a7b533a496eead6a1de211179fb/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java#L2208)|
   | solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java | [2797](https://github.com/epugh/solr/blob/97a53cb37ab30a7b533a496eead6a1de211179fb/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java#L2797)|
   | solr/test-framework/src/java/org/apache/solr/SolrJettyTestBase.java | [160](https://github.com/epugh/solr/blob/97a53cb37ab30a7b533a496eead6a1de211179fb/solr/test-framework/src/java/org/apache/solr/SolrJettyTestBase.java#L160)|
   | solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java | [2566](https://github.com/epugh/solr/blob/97a53cb37ab30a7b533a496eead6a1de211179fb/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java#L2566)|
   | solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java | [2195](https://github.com/epugh/solr/blob/97a53cb37ab30a7b533a496eead6a1de211179fb/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java#L2195)|
   <p><a href="https://lift.sonatype.com/results/github.com/apache/solr/01GF37GFS86SEAE2KC0MRA48R5?t=Infer|THREAD_SAFETY_VIOLATION" target="_blank">Visit the Lift Web Console</a> to find more details in your report.</p></div></details>
   
   
   
   ---
   
   <details><summary><b>ℹī¸ Learn about @sonatype-lift commands</b></summary>
   
   You can reply with the following commands. For example, reply with ***@sonatype-lift ignoreall*** to leave out all findings.
   | **Command** | **Usage** |
   | ------------- | ------------- |
   | `@sonatype-lift ignore` | Leave out the above finding from this PR |
   | `@sonatype-lift ignoreall` | Leave out all the existing findings from this PR |
   | `@sonatype-lift exclude <file\|issue\|path\|tool>` | Exclude specified `file\|issue\|path\|tool` from Lift findings by updating your config.toml file |
   
   **Note:** When talking to LiftBot, you need to **refresh** the page to see its response.
   <sub>[Click here](https://github.com/apps/sonatype-lift/installations/new) to add LiftBot to another repo.</sub></details>
   
   
   
   ---
   
   Was this a good recommendation?
   [ [🙁 Not relevant](https://www.sonatype.com/lift-comment-rating?comment=344018113&lift_comment_rating=1) ] - [ [😕 Won't fix](https://www.sonatype.com/lift-comment-rating?comment=344018113&lift_comment_rating=2) ] - [ [😑 Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=344018113&lift_comment_rating=3) ] - [ [🙂 Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=344018113&lift_comment_rating=4) ] - [ [😊 Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=344018113&lift_comment_rating=5) ]



-- 
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

[GitHub] [solr] epugh commented on pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
epugh commented on PR #1012:
URL: https://github.com/apache/solr/pull/1012#issuecomment-1247162304

   One thing I've noticed is a lot of places where we cast to a specific client just to get the httpClient.  We do it all over the place in the tests...
   
   ```
   solrClient = getCloudSolrClient(cluster);
   httpClient = (CloseableHttpClient) ((CloudLegacySolrClient) solrClient).getHttpClient();
   ```
   
   Thoughts on introducing a `getHttp1Client` method in the base test class that would take a `SolrClient` and could interrogate it and return the httpClient? That would remove a ton of places where we refer to `CloudLegacySolrClient`.


-- 
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

[GitHub] [solr] epugh commented on a diff in pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
epugh commented on code in PR #1012:
URL: https://github.com/apache/solr/pull/1012#discussion_r991222469


##########
solr/core/src/test/org/apache/solr/filestore/TestDistribPackageStore.java:
##########
@@ -192,10 +192,10 @@ public static void checkAllNodesForFile(
       assertResponseValues(10, new Fetcher(url, jettySolrRunner), expected);
 
       if (verifyContent) {
-        try (HttpSolrClient solrClient = (HttpSolrClient) jettySolrRunner.newClient()) {
+        try (HttpSolrClient httpSolrClient = (HttpSolrClient) jettySolrRunner.newClient()) {

Review Comment:
   maybe the way to go is to stick to `solrClient` everywhere, instead of `solrServer`, `solrHttpClient`, etc... unless the name is somehow helps you understand what is going on...    Part of this refactor is listerally to move to less specific subclasses...



-- 
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

[GitHub] [solr] epugh commented on a diff in pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
epugh commented on code in PR #1012:
URL: https://github.com/apache/solr/pull/1012#discussion_r992649981


##########
solr/core/src/test/org/apache/solr/filestore/TestDistribPackageStore.java:
##########
@@ -309,11 +309,10 @@ public static <T extends NavigableObject> T assertResponseValues(
   public static void uploadKey(byte[] bytes, String path, MiniSolrCloudCluster cluster)
       throws Exception {
     JettySolrRunner jetty = cluster.getRandomJetty(random());
-    try (HttpSolrClient client = (HttpSolrClient) jetty.newClient()) {
-      PackageUtils.uploadKey(
-          bytes, path, Paths.get(jetty.getCoreContainer().getSolrHome()), client);
+    try (HttpSolrClient solrClient = (HttpSolrClient) jetty.newClient()) {
+      PackageUtils.uploadKey(bytes, path, Paths.get(jetty.getCoreContainer().getSolrHome()));
       String url = jetty.getBaseURLV2() + "/node/files" + path + "?sync=true";
-      Object resp = Utils.executeGET(client.getHttpClient(), url, null);
+      Object resp = Utils.executeGET(solrClient.getHttpClient(), url, null);
       log.info("sync resp: {} was {}", url, resp);
     }

Review Comment:
   Yep.



-- 
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

[GitHub] [solr] sonatype-lift[bot] commented on pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on PR #1012:
URL: https://github.com/apache/solr/pull/1012#issuecomment-1272388321

   :warning: **311 God Classes** were detected by Lift in this project. [Visit the Lift web console](https://lift.sonatype.com/results/github.com/apache/solr/01GEWG4Z93QY9SG23PNWW42FKX?tab=technical-debt&utm_source=github.com&utm_campaign=lift-comment&utm_content=apache\%20solr) for more details.


-- 
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

[GitHub] [solr] epugh commented on a diff in pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
epugh commented on code in PR #1012:
URL: https://github.com/apache/solr/pull/1012#discussion_r991256010


##########
solr/core/src/java/org/apache/solr/util/ExportTool.java:
##########
@@ -490,7 +491,7 @@ class CoreHandler {
       }
 
       boolean exportDocsFromCore() throws IOException, SolrServerException {
-        HttpSolrClient client = new HttpSolrClient.Builder(baseurl).build();
+        SolrClient client = new HttpSolrClient.Builder(baseurl).build();

Review Comment:
   wonder if we should have a seperate issue to refactor to this!  we have this try/finally structure everywhere!



-- 
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

[GitHub] [solr] epugh merged pull request #1012: SOLR-16368: Use SolrClient type instead of overly specific subclasses

Posted by GitBox <gi...@apache.org>.
epugh merged PR #1012:
URL: https://github.com/apache/solr/pull/1012


-- 
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