Copilot commented on code in PR #4263:
URL: https://github.com/apache/texera/pull/4263#discussion_r2907463004
##########
amber/src/main/scala/org/apache/texera/amber/engine/common/Utils.scala:
##########
@@ -36,25 +35,52 @@ object Utils extends LazyLogging {
*
* @return the real absolute path to amber home directory
*/
+
+ import java.nio.file.{Files, Path, Paths}
+ import scala.jdk.CollectionConverters._
+ import scala.util.Using
+
lazy val amberHomePath: Path = {
val currentWorkingDirectory = Paths.get(".").toRealPath()
- // check if the current directory is the amber home path
+
if (isAmberHomePath(currentWorkingDirectory)) {
currentWorkingDirectory
} else {
- // from current path's parent directory, search its children to find
amber home path
- // current max depth is set to 2 (current path's siblings and direct
children)
- val searchChildren = Files
- .walk(currentWorkingDirectory.getParent, 2)
- .filter((path: Path) => isAmberHomePath(path))
- .findAny
- if (searchChildren.isPresent) {
- searchChildren.get
- } else {
+ val parent = Option(currentWorkingDirectory.getParent).getOrElse {
throw new RuntimeException(
- "Finding texera home path failed. Current working directory is " +
currentWorkingDirectory
+ s"Cannot search for texera home from filesystem root:
$currentWorkingDirectory"
)
}
+
+ // Pass 1: prefer the closest prefix (deepest ancestor) of
currentWorkingDirectory
+ val closestPrefix: Option[Path] =
+ Using.resource(Files.walk(parent, 2)) { stream =>
+ stream
+ .iterator()
+ .asScala
+ .filter(path => isAmberHomePath(path))
+ .map(_.toRealPath()) // normalize after filtering
+ .filter(path => path.startsWith(currentWorkingDirectory))
+ .maxByOption(_.getNameCount) // deepest prefix = closest ancestor
+ }
Review Comment:
The comment/variable name implies selecting the closest *prefix/ancestor* of
`currentWorkingDirectory`, but the predicate uses
`path.startsWith(currentWorkingDirectory)` which instead selects matches
*under* the working directory. If the intent is deepest ancestor (e.g., when
CWD is inside an `amber/...` subtree), the `startsWith` direction should be
reversed (or handle both ancestor/descendant cases explicitly) to avoid
selecting the wrong candidate in edge cases with multiple matches.
--
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]
