Re: [PR] [SPARK-46245][CORE][SQL][SS][YARN][K8S][UI] Replcace `s.c.MapOps.view.filterKeys` with `s.c.MapOps.filter` [spark]

2023-12-04 Thread via GitHub


LuciferYang commented on PR #44160:
URL: https://github.com/apache/spark/pull/44160#issuecomment-1839921427

   Thanks @dongjoon-hyun and @cloud-fan 


-- 
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-unsubscr...@spark.apache.org

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


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



Re: [PR] [SPARK-46245][CORE][SQL][SS][YARN][K8S][UI] Replcace `s.c.MapOps.view.filterKeys` with `s.c.MapOps.filter` [spark]

2023-12-04 Thread via GitHub


dongjoon-hyun commented on PR #44160:
URL: https://github.com/apache/spark/pull/44160#issuecomment-1839454529

   All tests passed. Merged to master for Apache Spark 4.0.0.


-- 
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-unsubscr...@spark.apache.org

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


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



Re: [PR] [SPARK-46245][CORE][SQL][SS][YARN][K8S][UI] Replcace `s.c.MapOps.view.filterKeys` with `s.c.MapOps.filter` [spark]

2023-12-04 Thread via GitHub


dongjoon-hyun closed pull request #44160: 
[SPARK-46245][CORE][SQL][SS][YARN][K8S][UI] Replcace 
`s.c.MapOps.view.filterKeys` with `s.c.MapOps.filter`
URL: https://github.com/apache/spark/pull/44160


-- 
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-unsubscr...@spark.apache.org

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


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



Re: [PR] [SPARK-46245][CORE][SQL][SS][YARN][K8S][UI] Replcace `s.c.MapOps.view.filterKeys` with `s.c.MapOps.filter` [spark]

2023-12-04 Thread via GitHub


LuciferYang commented on PR #44160:
URL: https://github.com/apache/spark/pull/44160#issuecomment-1839216786

   Some flaky test failed, I will re-trigger them later


-- 
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-unsubscr...@spark.apache.org

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


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



Re: [PR] [SPARK-46245][CORE][SQL][SS][YARN][K8S][UI] Replcace `s.c.MapOps.view.filterKeys` with `s.c.MapOps.filter` [spark]

2023-12-04 Thread via GitHub


cloud-fan commented on code in PR #44160:
URL: https://github.com/apache/spark/pull/44160#discussion_r1414253928


##
core/src/main/scala/org/apache/spark/scheduler/cluster/StandaloneSchedulerBackend.scala:
##
@@ -254,8 +254,8 @@ private[spark] class StandaloneSchedulerBackend(
 
   override def getDriverLogUrls: Option[Map[String, String]] = {
 val prefix = "SPARK_DRIVER_LOG_URL_"
-val driverLogUrls = sys.env.view.filterKeys(_.startsWith(prefix))
-  .map(e => (e._1.substring(prefix.length).toLowerCase(Locale.ROOT), 
e._2)).toMap
+val driverLogUrls = sys.env.filter { case (k, _) => k.startsWith(prefix) }

Review Comment:
   I don't have a strong opinion. Both are fine.



-- 
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-unsubscr...@spark.apache.org

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


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



Re: [PR] [SPARK-46245][CORE][SQL][SS][YARN][K8S][UI] Replcace `s.c.MapOps.view.filterKeys` with `s.c.MapOps.filter` [spark]

2023-12-04 Thread via GitHub


LuciferYang commented on PR #44160:
URL: https://github.com/apache/spark/pull/44160#issuecomment-1839107996

   > Are we going to remove all .view.?
   
   Yes, for ·MapOps.view·, this last pr, the reason for making this pr can be 
referred to https://github.com/apache/spark/pull/43445#discussion_r1413338062. 
   
   > How many PRs do we need?
   
   As for the remaining `Seq.view` and `Array.view` in the spark code, they 
were not introduced by the recent refactoring work, 
   I am investigating their necessity now. If cleanup is needed, it will be 
completed in one.
   
   


-- 
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-unsubscr...@spark.apache.org

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


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



Re: [PR] [SPARK-46245][CORE][SQL][SS][YARN][K8S][UI] Replcace `s.c.MapOps.view.filterKeys` with `s.c.MapOps.filter` [spark]

2023-12-04 Thread via GitHub


LuciferYang commented on code in PR #44160:
URL: https://github.com/apache/spark/pull/44160#discussion_r1413845939


##
core/src/main/scala/org/apache/spark/scheduler/cluster/StandaloneSchedulerBackend.scala:
##
@@ -254,8 +254,8 @@ private[spark] class StandaloneSchedulerBackend(
 
   override def getDriverLogUrls: Option[Map[String, String]] = {
 val prefix = "SPARK_DRIVER_LOG_URL_"
-val driverLogUrls = sys.env.view.filterKeys(_.startsWith(prefix))
-  .map(e => (e._1.substring(prefix.length).toLowerCase(Locale.ROOT), 
e._2)).toMap
+val driverLogUrls = sys.env.filter { case (k, _) => k.startsWith(prefix) }

Review Comment:
   cc @cloud-fan FYI
   
   Although I prefer the current change, if you lean towards a style like 
`sys.env.filter(kv => kv._1.startsWith(prefix))` or have another preferred 
style, please let me know, and I can fix it again.



-- 
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-unsubscr...@spark.apache.org

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


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