Github user HeartSaVioR commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21721#discussion_r206009928
  
    --- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/ProgressReporter.scala
 ---
    @@ -143,18 +150,50 @@ trait ProgressReporter extends Logging {
         }
         logDebug(s"Execution stats: $executionStats")
     
    +    // extracts custom metrics from readers and writers
    +    def extractMetrics(getMetrics: () => CustomMetrics,
    +                      onInvalidMetrics: (Exception) => Unit): 
Option[JValue] = {
    +      val metrics = getMetrics()
    +      if (metrics != null) {
    +        try {
    +          Some(parse(metrics.json()))
    +        } catch {
    +          case ex: Exception => onInvalidMetrics(ex)
    --- End diff --
    
    
https://github.com/databricks/scala-style-guide#exception-handling-try-vs-try
    
    According to the guide, this line needs to be replaced with `case 
NonFatal(e) =>`, and I'd place `onInvalidMetrics` and `None` to same 
indentation.


---

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

Reply via email to