[GitHub] HeartSaVioR commented on a change in pull request #23790: [SPARK-26792][CORE] Apply custom log URL to Spark UI

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

HeartSaVioR commented on a change in pull request #23790: [SPARK-26792][CORE] Apply custom log URL to Spark UI
URL: https://github.com/apache/spark/pull/23790#discussion_r256844670
 
 

 ##########
 File path: core/src/main/scala/org/apache/spark/deploy/history/HistoryAppStatusStore.scala
 ##########
 @@ -64,56 +59,10 @@ private[spark] class HistoryAppStatusStore(
   }
 
   private def replaceLogUrls(exec: v1.ExecutorSummary, urlPattern: String): v1.ExecutorSummary = {
-    val attributes = exec.attributes
-
-    // Relation between pattern {{FILE_NAME}} and attribute {{LOG_FILES}}
-    // Given that HistoryAppStatusStore don't know which types of log files can be provided
-    // from resource manager, we require resource manager to provide available types of log
-    // files, which are encouraged to be same as types of log files provided in original log URLs.
-    // Once we get the list of log files, we need to expose them to end users as a pattern
-    // so that end users can compose custom log URL(s) including log file name(s).
-    val allPatterns = CUSTOM_URL_PATTERN_REGEX.findAllMatchIn(urlPattern).map(_.group(1)).toSet
-    val allPatternsExceptFileName = allPatterns.filter(_ != "FILE_NAME")
-    val allAttributeKeys = attributes.keySet
-    val allAttributeKeysExceptLogFiles = allAttributeKeys.filter(_ != "LOG_FILES")
-
-    if (allPatternsExceptFileName.diff(allAttributeKeysExceptLogFiles).nonEmpty) {
-      logFailToRenewLogUrls("some of required attributes are missing in app's event log.",
-        allPatternsExceptFileName, allAttributeKeys)
-      return exec
-    } else if (allPatterns.contains("FILE_NAME") && !allAttributeKeys.contains("LOG_FILES")) {
-      logFailToRenewLogUrls("'FILE_NAME' parameter is provided, but file information is " +
-        "missing in app's event log.", allPatternsExceptFileName, allAttributeKeys)
-      return exec
-    }
-
-    val updatedUrl = allPatternsExceptFileName.foldLeft(urlPattern) { case (orig, patt) =>
-      // we already checked the existence of attribute when comparing keys
-      orig.replace(s"{{$patt}}", attributes(patt))
-    }
-
-    val newLogUrlMap = if (allPatterns.contains("FILE_NAME")) {
-      // allAttributeKeys should contain "LOG_FILES"
-      attributes("LOG_FILES").split(",").map { file =>
-        file -> updatedUrl.replace("{{FILE_NAME}}", file)
-      }.toMap
-    } else {
-      Map("log" -> updatedUrl)
-    }
-
+    val newLogUrlMap = logUrlHandler.applyPattern(exec.executorLogs, exec.attributes)
 
 Review comment:
   All major logics for handling log url pattern are moved to `ExecutorLogUrlHandler` to let both of Spark UI and SHS leverage it together.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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