dsmiley commented on code in PR #2741:
URL: https://github.com/apache/solr/pull/2741#discussion_r1804727277


##########
solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java:
##########
@@ -180,23 +184,35 @@ private boolean fetchFileFromNodeAndPersist(String 
fromNode) {
 
       ByteBuffer metadata = null;
       Map<?, ?> m = null;
+
+      InputStream is = null;
+      var solrClient = coreContainer.getDefaultHttpSolrClient();
+
       try {
+        GenericSolrRequest request = new GenericSolrRequest(GET, "/node/files" 
+ getMetaPath());
+        request.setResponseParser(new InputStreamResponseParser(null));
+        var response = solrClient.requestWithBaseUrl(baseUrl, 
request::process).getResponse();
+        is = (InputStream) response.get("stream");
         metadata =
-            Utils.executeGET(
-                coreContainer.getUpdateShardHandler().getDefaultHttpClient(),
-                baseUrl + "/node/files" + getMetaPath(),
-                Utils.newBytesConsumer((int) MAX_PKG_SIZE));
+            Utils.newBytesConsumer((int) MAX_PKG_SIZE).accept((InputStream) 
response.get("stream"));
         m = (Map<?, ?>) Utils.fromJSON(metadata.array(), 
metadata.arrayOffset(), metadata.limit());

Review Comment:
   Please replace needless use of the special InputStreamResponseParser with a 
standard JsonMapResponseParser.  Perhaps don't even do that; we don't have to 
use JSON, I think; Solr will by default negotiate the format and get you a 
NamedList. (Map like thing).



##########
solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java:
##########
@@ -221,17 +242,18 @@ boolean fetchFromAnyNode() {
         try {
           String baseUrl =
               
coreContainer.getZkController().getZkStateReader().getBaseUrlV2ForNodeName(liveNode);
-          String reqUrl = baseUrl + "/node/files" + path + 
"?meta=true&wt=javabin&omitHeader=true";
+          final var solrParams = new ModifiableSolrParams();
+          solrParams.add("meta", "true");
+          solrParams.add("omitHeader", "true");
+
+          final var request = new GenericSolrRequest(GET, "/node/files" + 
path, solrParams);
           boolean nodeHasBlob = false;
-          Object nl =
-              Utils.executeGET(
-                  coreContainer.getUpdateShardHandler().getDefaultHttpClient(),
-                  reqUrl,
-                  Utils.JAVABINCONSUMER);
-          if (Utils.getObjectByPath(nl, false, Arrays.asList("files", path)) 
!= null) {
+          var solrClient = coreContainer.getDefaultHttpSolrClient();
+          var resp = solrClient.requestWithBaseUrl(baseUrl, 
request::process).getResponse();
+
+          if (Utils.getObjectByPath(resp, false, Arrays.asList("files", path)) 
!= null) {

Review Comment:
   there are convenience methods on NamedList avoiding the need to refer to 
this awkward utility method.  Try findRecursive.



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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to