risdenk commented on code in PR #1239:
URL: https://github.com/apache/solr/pull/1239#discussion_r1048817209
##########
solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java:
##########
@@ -238,9 +238,14 @@ public List<String> getAllConfigFiles(String configName)
throws IOException {
@Override
public FileVisitResult visitFile(Path file, BasicFileAttributes
attrs)
throws IOException {
- // don't include hidden (.) files
- if (!Files.isHidden(file)) {
- filePaths.add(configDir.relativize(file).toString());
+ Path filePath = configDir.relativize(file);
+ String filePathStr = filePath.toString();
+ filePathStr =
+ filePathStr.replace(
+ filePath.getFileSystem().getSeparator(), "/"); //
normalize slashes
+ // don't include .metadata.json hidden file
+ if (!filePathStr.equals(METADATA_FILE)) {
+ filePaths.add(filePathStr);
Review Comment:
Hmmm I see what you mean about `List<String>` I guess I'm confused maybe by
the variable name and what is going on here. I haven't looked at the original
PR or the whole change just related to this error.
I'll try to lay out my thoughts and maybe there is some misunderstanding I
have.
* normalizing paths to `/` - if we do this - then on Windows this is no
longer a filesystem path right? We are forcing these to be what? just strings
to compare?
* In my head, we don't need to normalize paths - since
`FileSystemConfigSetService` is run per node. Even if you had a mixed
Windows/Linux Solr setup - each node (either Windows or Linux) would only care
about file paths in its format (`/` or `\`). The strings being compared should
just be comparable regardless of this "normalizing" - if its a path on windows
- who cares if its `\` separating.
* We are looking at filePathStr for `METADATA_FILE` - which should be just a
`Path` comparison - regardless of what ends up in `filePaths` right?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
