demery-pivotal commented on a change in pull request #5595:
URL: https://github.com/apache/geode/pull/5595#discussion_r500649130
##########
File path:
geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java
##########
@@ -181,12 +180,13 @@ private long filterAndSize(Path originalLogFile) throws
IOException {
}
private List<Path> findFiles(Path workingDir, Predicate<Path> fileSelector)
throws IOException {
- Stream<Path> selectedFiles/* = null */;
if (!workingDir.toFile().isDirectory()) {
return Collections.emptyList();
}
- selectedFiles =
Files.list(workingDir).filter(fileSelector).filter(this.logFilter::acceptsFile);
-
- return selectedFiles.collect(toList());
+ return Files.list(workingDir)
+ .filter(Files::isRegularFile)
Review comment:
Yes, that's what I thought, and I verified with some experiments (which
did not involve the exporter).
My question remains: If the path we're testing is a symbolic link, are you
saying that you _want_ to ignore the file it links to? If so, what makes you
want to skip the file in that case?
One possibility: The link links to a file in the same dir, which would lead
us to export the content twice (once via the file, and once via the link to the
file). We could solve this by resolving the real path to the file, and
collecting the matching files into a Set instead of a List.
Another possibility: The link links to a file in some other directory, and
some concern makes you not want to read the file from the other directory.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
