You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "risdenk (via GitHub)" <gi...@apache.org> on 2023/03/24 15:09:45 UTC

[GitHub] [solr] risdenk opened a new pull request, #1491: SOLR-16716: Replace commons-io usages with pure Java

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

   https://issues.apache.org/jira/browse/SOLR-16716


-- 
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] risdenk commented on a diff in pull request #1491: SOLR-16716: Replace commons-io usages with pure Java

Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1491:
URL: https://github.com/apache/solr/pull/1491#discussion_r1148410861


##########
solr/core/src/test/org/apache/solr/schema/ChangedSchemaMergeTest.java:
##########
@@ -83,15 +84,15 @@ private void addDoc(SolrCore core, String... fieldValues) throws IOException {
   }
 
   private CoreContainer init() throws Exception {
-    File changed = new File(solrHomeDirectory, "changed");
-    copyMinConf(changed, "name=changed");
+    Path changed = solrHomeDirectory.toPath().resolve("changed");
+    copyMinConf(changed.toFile(), "name=changed");
     // Overlay with my local schema
-    schemaFile = new File(new File(changed, "conf"), "schema.xml");
-    FileUtils.writeStringToFile(schemaFile, withWhich, StandardCharsets.UTF_8);
+    schemaFile = changed.resolve("conf").resolve("schema.xml").toFile();
+    Files.writeString(schemaFile.toPath(), withWhich, StandardCharsets.UTF_8);

Review Comment:
   gah good call. I figured schemaFile and solrHomeDirectory were part of the test framework and not private variables.



-- 
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] risdenk commented on a diff in pull request #1491: SOLR-16716: Replace commons-io usages with pure Java

Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1491:
URL: https://github.com/apache/solr/pull/1491#discussion_r1148411312


##########
solr/core/src/test/org/apache/solr/schema/ChangedSchemaMergeTest.java:
##########
@@ -83,15 +84,15 @@ private void addDoc(SolrCore core, String... fieldValues) throws IOException {
   }
 
   private CoreContainer init() throws Exception {
-    File changed = new File(solrHomeDirectory, "changed");
-    copyMinConf(changed, "name=changed");
+    Path changed = solrHomeDirectory.toPath().resolve("changed");
+    copyMinConf(changed.toFile(), "name=changed");
     // Overlay with my local schema
-    schemaFile = new File(new File(changed, "conf"), "schema.xml");
-    FileUtils.writeStringToFile(schemaFile, withWhich, StandardCharsets.UTF_8);
+    schemaFile = changed.resolve("conf").resolve("schema.xml").toFile();
+    Files.writeString(schemaFile.toPath(), withWhich, StandardCharsets.UTF_8);

Review Comment:
   Fixed in 2d79b286b652e6e5da265a9e3767252fa7063532



-- 
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] risdenk commented on pull request #1491: SOLR-16716: Replace commons-io usages with pure Java

Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on PR #1491:
URL: https://github.com/apache/solr/pull/1491#issuecomment-1483885157

   > Maybe block org.apache.commons.io.FileUtils altogether. It's java.io.File oriented; we want Path nowadays.
   
   yea agreed I just didn't want to tackle that all at once. FileUtils moves to PathUtils for almost all remaining cases, but requires quite a few changes to go from File -> Path. https://issues.apache.org/jira/browse/SOLR-15919 started w/ Mike converting a few things. I'll tackle switching File -> Path separately.


-- 
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] risdenk merged pull request #1491: SOLR-16716: Replace commons-io usages with pure Java

Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk merged PR #1491:
URL: https://github.com/apache/solr/pull/1491


-- 
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 #1491: SOLR-16716: Replace commons-io usages with pure Java

Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #1491:
URL: https://github.com/apache/solr/pull/1491#discussion_r1148278758


##########
solr/core/src/test/org/apache/solr/schema/ChangedSchemaMergeTest.java:
##########
@@ -83,15 +84,15 @@ private void addDoc(SolrCore core, String... fieldValues) throws IOException {
   }
 
   private CoreContainer init() throws Exception {
-    File changed = new File(solrHomeDirectory, "changed");
-    copyMinConf(changed, "name=changed");
+    Path changed = solrHomeDirectory.toPath().resolve("changed");
+    copyMinConf(changed.toFile(), "name=changed");
     // Overlay with my local schema
-    schemaFile = new File(new File(changed, "conf"), "schema.xml");
-    FileUtils.writeStringToFile(schemaFile, withWhich, StandardCharsets.UTF_8);
+    schemaFile = changed.resolve("conf").resolve("schema.xml").toFile();
+    Files.writeString(schemaFile.toPath(), withWhich, StandardCharsets.UTF_8);

Review Comment:
   The fields `solrHomeDirectory` and `schemaFile` should both be changed to be of type Path.  I took a look and we're always using them where we actually want a Path.



-- 
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 #1491: SOLR-16716: Replace commons-io usages with pure Java

Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #1491:
URL: https://github.com/apache/solr/pull/1491#discussion_r1147963970


##########
solr/core/src/java/org/apache/solr/cloud/api/collections/TimeRoutedAlias.java:
##########


Review Comment:
   I like getting rid of the werid MoreObjects class that I've never seen before, boy I wish Java had nicer string building ;-)



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