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


##########
solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java:
##########
@@ -189,26 +187,32 @@ private boolean fetchFileFromNodeAndPersist(String 
fromNode) {
       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");
+        final var metadataRequest = new FileStoreApi.GetFile(getMetaPath());
+        final var client = 
coreContainer.getSolrClientCache().getHttpSolrClient(baseUrl);
+        final var response = metadataRequest.process(client);
         metadata =
-            Utils.newBytesConsumer((int) MAX_PKG_SIZE).accept((InputStream) 
response.get("stream"));
+            Utils.newBytesConsumer((int) MAX_PKG_SIZE)
+                .accept(response.getResponseStreamIfSuccessful());
         m = (Map<?, ?>) Utils.fromJSON(metadata.array(), 
metadata.arrayOffset(), metadata.limit());
-      } catch (SolrServerException | IOException e) {
+      } catch (Exception e) {
         throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Error 
fetching metadata", e);
       } finally {
         org.apache.solr.common.util.IOUtils.closeQuietly(is);
       }
 
+      ByteBuffer filedata = null;
+      try {
+        final var fileRequest = new FileStoreApi.GetFile(path);
+        final var client = 
coreContainer.getSolrClientCache().getHttpSolrClient(baseUrl);

Review Comment:
   Please don't use the SolrClientCache for anything other than streaming 
expressions.  I'm stamping out such usages here: 
https://issues.apache.org/jira/browse/SOLR-17630
   I'm particularly surprised to see you removed a use of the new 
`requestWithBaseUrl`.



##########
solr/core/src/java/org/apache/solr/filestore/ClusterFileStore.java:
##########
@@ -194,6 +301,48 @@ public SolrJerseyResponse deleteFile(String filePath, 
Boolean localDelete) {
     return response;
   }
 
+  @Override
+  @PermissionName(PermissionNameProvider.Name.FILESTORE_WRITE_PERM)
+  public SolrJerseyResponse executeFileStoreCommand(String path, String 
getFrom, Boolean sync) {
+    final var response = instantiateJerseyResponse(SolrJerseyResponse.class);
+
+    if (Boolean.TRUE.equals(sync)) {
+      syncToAllNodes(fileStore, path);
+    } else if (getFrom != null) {
+      if (path == null) {
+        path = "";
+      }
+      pullFileFromNode(coreContainer, fileStore, path, getFrom);
+    }
+
+    return response;
+  }
+
+  public static void syncToAllNodes(FileStore fileStore, String path) {
+    try {
+      fileStore.syncToAllNodes(path);
+    } catch (IOException e) {
+      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Error 
getting file ", e);

Review Comment:
   We don't know if we can blame the user



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