HeartSaVioR commented on a change in pull request #26723: [SPARK-27523][CORE] - 
Resolve scheme-less event log directory relative to default filesystem
URL: https://github.com/apache/spark/pull/26723#discussion_r352289699
 
 

 ##########
 File path: core/src/main/scala/org/apache/spark/SparkContext.scala
 ##########
 @@ -509,6 +501,17 @@ class SparkContext(config: SparkConf) extends Logging {
       files.foreach(addFile)
     }
 
+    _eventLogDir =
+      if (isEventLogEnabled) {
+        val defaultFSProperty = _hadoopConfiguration.get("fs.defaultFS")
+        val defaultFS = if (defaultFSProperty == null) "" else 
defaultFSProperty
+
+        val unresolvedDir = 
s"$defaultFS${conf.get(EVENT_LOG_DIR).stripSuffix("/")}"
 
 Review comment:
   I'm seeing the needs of this patch, as Utils.resolveURI just uses `file` 
scheme if there's no scheme, though we may want to handle the case where the 
scheme is specified in the value of EVENT_LOG_DIR.
   
   This would handle the scheme-less case as well as relative case:
   
   ```
   val fs = new Path(unresolvedDir).getFileSystem(_hadoopConfiguration)
   val qualifiedPath = fs.makeQualified(new Path(unresolvedDir))
   Some(qualifiedPath.toUri)
   ```
   
   Btw, can we add some tests to check the possible cases for _eventLogDir? You 
can find some existing FileSystem implementations like FakeFileSystem in Spark 
code to add tests without HDFS.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to