[GitHub] [spark] xkrogen commented on a change in pull request #35688: [SPARK-38356][YARN] Print resolved versions of platform JARs used by Spark

[GitBox <gi...@apache.org>]

xkrogen commented on a change in pull request #35688:
URL: https://github.com/apache/spark/pull/35688#discussion_r816346463



##########
File path: resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala
##########
@@ -447,7 +447,8 @@ private[spark] class Client(
     var destPath = srcPath
     if (force || !compareFs(srcFs, destFs) || "file".equals(srcFs.getScheme)) {
       destPath = new Path(destDir, destName.getOrElse(srcPath.getName()))
-      logInfo(s"Uploading resource $srcPath -> $destPath")
+      val resolvedSrcPath = new Path(Paths.get(srcPath.toUri.getPath).toRealPath().toUri)

Review comment:
       Unfortunately this won't work well if the filesystem is not the local filesystem. We'll enter this branch if the filesystem is local (`"file".equals(srcFs.getScheme)`), but we'll also get here if the src and dst FS are different (`!compareFs(srcFs, destFs)`), so srcFS could be anything: HDFS, ADLS, S3, etc. In those cases this logic won't work. We probably have to add another check for `"file".equals(srcFs.getScheme)` and only use the NIO APIs in that case.
   
   This actually made me wonder if we can use the `FileSystem#resolvePath` method from the Hadoop FS API? I'm not sure if it resolves local-FS symlinks or not, but if it does, it would nicely avoid the problem mentioned above.

##########
File path: resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala
##########
@@ -447,7 +447,8 @@ private[spark] class Client(
     var destPath = srcPath
     if (force || !compareFs(srcFs, destFs) || "file".equals(srcFs.getScheme)) {
       destPath = new Path(destDir, destName.getOrElse(srcPath.getName()))
-      logInfo(s"Uploading resource $srcPath -> $destPath")
+      val resolvedSrcPath = new Path(Paths.get(srcPath.toUri.getPath).toRealPath().toUri)
+      logInfo(s"Uploading resource $resolvedSrcPath -> $destPath")

Review comment:
       Maybe we should print both the original version and the resolved version? (if they are different)




-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org