You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@toree.apache.org by "vincentye38 (via GitHub)" <gi...@apache.org> on 2025/09/05 05:26:42 UTC
[PR] Jupyter Spark monitor [incubator-toree]
vincentye38 opened a new pull request, #229:
URL: https://github.com/apache/incubator-toree/pull/229
Implement a spark-monitor plugin to sent spark progress update to [SparkMonitor](https://krishnan-r.github.io/sparkmonitor/) frontend extension.
This plugin can be built as individual jar and enable by specifying --magic-url" args[7] = "file:///path/to/spark-monitor-plugin.jar"
--
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@toree.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Jupyter Spark monitor [incubator-toree]
Posted by "lresende (via GitHub)" <gi...@apache.org>.
lresende commented on PR #229:
URL: https://github.com/apache/incubator-toree/pull/229#issuecomment-3369994214
If no other comments, I will merge this tomorrow.
--
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@toree.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Jupyter Spark monitor [incubator-toree]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #229:
URL: https://github.com/apache/incubator-toree/pull/229#discussion_r2409085681
##########
spark-monitor-plugin/src/main/scala/org/apache/toree/plugins/sparkmonitor/JupyterSparkMonitorListener.scala:
##########
@@ -0,0 +1,781 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+
+package org.apache.toree.plugins.sparkmonitor
+
+import org.apache.spark.scheduler._
+import play.api.libs.json._
+import org.apache.spark._
+import org.apache.spark.TaskEndReason
+import org.apache.spark.JobExecutionStatus
+import org.apache.spark.SparkContext
+import org.apache.log4j.Logger
Review Comment:
Use SLF4J Logger
--
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@toree.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Jupyter Spark monitor [incubator-toree]
Posted by "vincentye38 (via GitHub)" <gi...@apache.org>.
vincentye38 commented on code in PR #229:
URL: https://github.com/apache/incubator-toree/pull/229#discussion_r2417939775
##########
plugins/build.sbt:
##########
@@ -21,7 +21,10 @@ Test / fork := true
libraryDependencies ++= Seq(
Dependencies.scalaReflect.value,
Dependencies.clapper,
- Dependencies.slf4jApi
+ Dependencies.slf4jApi,
+ "net.sf.py4j" % "py4j" % "0.10.7" % "provided",
+ "org.json4s" % "json4s-native_2.12" % "3.7.0-M11" % "provided",
Review Comment:
This plug is built separately and not being in the assembly. Do I still need to move these to `project/Dependencies.scala` ?
--
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@toree.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Jupyter Spark monitor [incubator-toree]
Posted by "vincentye38 (via GitHub)" <gi...@apache.org>.
vincentye38 commented on PR #229:
URL: https://github.com/apache/incubator-toree/pull/229#issuecomment-3351058194
@lresende Any update?
--
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@toree.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Jupyter Spark monitor [incubator-toree]
Posted by "lresende (via GitHub)" <gi...@apache.org>.
lresende commented on PR #229:
URL: https://github.com/apache/incubator-toree/pull/229#issuecomment-3335412931
@vincentye38, when you rebase this pr it should re-run all the tests 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-unsubscribe@toree.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Jupyter Spark monitor [incubator-toree]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #229:
URL: https://github.com/apache/incubator-toree/pull/229#issuecomment-5712148185
See #282 - some of this code appears to have been borrowed from https://github.com/swan-cern/sparkmonitor/blob/c12c0bc259eff9ad936971a616a08ab584138d10/scalalistener_spark3/CustomListener.scala
--
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@toree.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Jupyter Spark monitor [incubator-toree]
Posted by "lresende (via GitHub)" <gi...@apache.org>.
lresende commented on PR #229:
URL: https://github.com/apache/incubator-toree/pull/229#issuecomment-3257433224
Thank you for the contribution @vincentye38
--
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@toree.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Jupyter Spark monitor [incubator-toree]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #229:
URL: https://github.com/apache/incubator-toree/pull/229#discussion_r2409088764
##########
spark-monitor-plugin/src/main/scala/org/apache/toree/plugins/sparkmonitor/JupyterSparkMonitorListener.scala:
##########
@@ -0,0 +1,781 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+
+package org.apache.toree.plugins.sparkmonitor
+
+import org.apache.spark.scheduler._
+import play.api.libs.json._
Review Comment:
you pull json4s deps, but use play json? if possible, use the spark builtin json libs as much as possible - jackson or json4s
--
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@toree.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Jupyter Spark monitor [incubator-toree]
Posted by "vincentye38 (via GitHub)" <gi...@apache.org>.
vincentye38 commented on PR #229:
URL: https://github.com/apache/incubator-toree/pull/229#issuecomment-3378096640
Rebased on https://github.com/apache/incubator-toree/pull/218
--
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@toree.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Jupyter Spark monitor [incubator-toree]
Posted by "vincentye38 (via GitHub)" <gi...@apache.org>.
vincentye38 commented on PR #229:
URL: https://github.com/apache/incubator-toree/pull/229#issuecomment-3373334003
> Hi @vincentye38, some things I noticed :
>
> * [x] make commands were not building the jar automatically
> * [x] jar names were not being prefixed with `toree`
> * [x] rewrote the JSON parser using playjson to avoid adding new dependencies
> * [] Magic URL is a bit strange, but I understand this is something that was already there
>
> <img alt="image" width="1481" height="417" src="https://private-user-images.githubusercontent.com/382917/497649442-93aac96b-2864-402d-92e9-1a70e8aa853a.png?jwt=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NTk3NzYzMTgsIm5iZiI6MTc1OTc3NjAxOCwicGF0aCI6Ii8zODI5MTcvNDk3NjQ5NDQyLTkzYWFjOTZiLTI4NjQtNDAyZC05MmU5LTFhNzBlOGFhODUzYS5wbmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjUxMDA2JTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI1MTAwNlQxODQwMThaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT0xMWI1YjJiZTFkMzliMTlkNzk0ZjhhMjZlMzUzMTNiNzk0NWMwYjZiYmZlMTlmZTcyNTljMDliMTI0Yzc3NzkyJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCJ9.L-X-8FfRwkUbIF4E7NWsX1UBcfcbey09iD0WNkOGZdI">
Thank you @lresende for the enhancement. They looks good to me.
--
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@toree.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Jupyter Spark monitor [incubator-toree]
Posted by "vincentye38 (via GitHub)" <gi...@apache.org>.
vincentye38 commented on PR #229:
URL: https://github.com/apache/incubator-toree/pull/229#issuecomment-3314319622
> @vincentye38 could you take a look in the build failures?
How to trigger the PR build again? looks like the build error is not related to my code change.
--
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@toree.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Jupyter Spark monitor [incubator-toree]
Posted by "lresende (via GitHub)" <gi...@apache.org>.
lresende commented on PR #229:
URL: https://github.com/apache/incubator-toree/pull/229#issuecomment-3369991807
Hi @vincentye38, some things I noticed :
- [x] make commands were not building the jar automatically
- [x] jar names were not being prefixed with `toree`
- [x] rewrote the JSON parser using playjson to avoid adding new dependencies
- [] Magic URL is a bit strange, but I understand this is something that was already there
<img width="1481" height="417" alt="image" src="https://github.com/user-attachments/assets/93aac96b-2864-402d-92e9-1a70e8aa853a" />
--
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@toree.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Jupyter Spark monitor [incubator-toree]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #229:
URL: https://github.com/apache/incubator-toree/pull/229#issuecomment-3374856989
@vincentye38 FYI, now Toree also supports Scala 2.13
- https://github.com/apache/incubator-toree/pull/218
Could you rebase this PR on the latest master, and make sure the patch works with both Scala 2.12 and 2.13?
--
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@toree.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Jupyter Spark monitor [incubator-toree]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #229:
URL: https://github.com/apache/incubator-toree/pull/229#discussion_r2409084941
##########
plugins/build.sbt:
##########
@@ -21,7 +21,10 @@ Test / fork := true
libraryDependencies ++= Seq(
Dependencies.scalaReflect.value,
Dependencies.clapper,
- Dependencies.slf4jApi
+ Dependencies.slf4jApi,
+ "net.sf.py4j" % "py4j" % "0.10.7" % "provided",
+ "org.json4s" % "json4s-native_2.12" % "3.7.0-M11" % "provided",
Review Comment:
this should be defined at `project/Dependencies.scala`, and
```patch
- "org.json4s" % "json4s-native_2.12" % "3.7.0-M11" % "provided",
+ "org.json4s" % "json4s-native" %% "3.7.0-M11" % "provided",
```
--
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@toree.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Jupyter Spark monitor [incubator-toree]
Posted by "vincentye38 (via GitHub)" <gi...@apache.org>.
vincentye38 commented on code in PR #229:
URL: https://github.com/apache/incubator-toree/pull/229#discussion_r2417939775
##########
plugins/build.sbt:
##########
@@ -21,7 +21,10 @@ Test / fork := true
libraryDependencies ++= Seq(
Dependencies.scalaReflect.value,
Dependencies.clapper,
- Dependencies.slf4jApi
+ Dependencies.slf4jApi,
+ "net.sf.py4j" % "py4j" % "0.10.7" % "provided",
+ "org.json4s" % "json4s-native_2.12" % "3.7.0-M11" % "provided",
Review Comment:
This plug is built separately and not being in the assembly. Do I still need to move these to `project/Dependencies.scala` ?
--
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@toree.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Jupyter Spark monitor [incubator-toree]
Posted by "lresende (via GitHub)" <gi...@apache.org>.
lresende commented on code in PR #229:
URL: https://github.com/apache/incubator-toree/pull/229#discussion_r2418104643
##########
plugins/build.sbt:
##########
@@ -21,7 +21,8 @@ Test / fork := true
libraryDependencies ++= Seq(
Dependencies.scalaReflect.value,
Dependencies.clapper,
- Dependencies.slf4jApi
+ Dependencies.slf4jApi,
+ Dependencies.py4j,
Review Comment:
Why are we adding the py4j dependency here?
--
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@toree.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Jupyter Spark monitor [incubator-toree]
Posted by "lresende (via GitHub)" <gi...@apache.org>.
lresende commented on PR #229:
URL: https://github.com/apache/incubator-toree/pull/229#issuecomment-3282053392
@vincentye38 could you take a look in the build failures?
--
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@toree.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Jupyter Spark monitor [incubator-toree]
Posted by "vincentye38 (via GitHub)" <gi...@apache.org>.
vincentye38 commented on PR #229:
URL: https://github.com/apache/incubator-toree/pull/229#issuecomment-3336968251
thank you. The PR build is passed.
--
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@toree.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Jupyter Spark monitor [incubator-toree]
Posted by "vincentye38 (via GitHub)" <gi...@apache.org>.
vincentye38 commented on code in PR #229:
URL: https://github.com/apache/incubator-toree/pull/229#discussion_r2418024626
##########
spark-monitor-plugin/src/main/scala/org/apache/toree/plugins/sparkmonitor/JupyterSparkMonitorListener.scala:
##########
@@ -0,0 +1,781 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+
+package org.apache.toree.plugins.sparkmonitor
+
+import org.apache.spark.scheduler._
+import play.api.libs.json._
Review Comment:
removed json4s dependencies from build.sbt.
##########
spark-monitor-plugin/src/main/scala/org/apache/toree/plugins/sparkmonitor/JupyterSparkMonitorListener.scala:
##########
@@ -0,0 +1,781 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+
+package org.apache.toree.plugins.sparkmonitor
+
+import org.apache.spark.scheduler._
+import play.api.libs.json._
+import org.apache.spark._
+import org.apache.spark.TaskEndReason
+import org.apache.spark.JobExecutionStatus
+import org.apache.spark.SparkContext
+import org.apache.log4j.Logger
Review Comment:
fixed
##########
plugins/build.sbt:
##########
@@ -21,7 +21,10 @@ Test / fork := true
libraryDependencies ++= Seq(
Dependencies.scalaReflect.value,
Dependencies.clapper,
- Dependencies.slf4jApi
+ Dependencies.slf4jApi,
+ "net.sf.py4j" % "py4j" % "0.10.7" % "provided",
+ "org.json4s" % "json4s-native_2.12" % "3.7.0-M11" % "provided",
Review Comment:
fixed
--
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@toree.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Jupyter Spark monitor [incubator-toree]
Posted by "vincentye38 (via GitHub)" <gi...@apache.org>.
vincentye38 commented on code in PR #229:
URL: https://github.com/apache/incubator-toree/pull/229#discussion_r2417939775
##########
plugins/build.sbt:
##########
@@ -21,7 +21,10 @@ Test / fork := true
libraryDependencies ++= Seq(
Dependencies.scalaReflect.value,
Dependencies.clapper,
- Dependencies.slf4jApi
+ Dependencies.slf4jApi,
+ "net.sf.py4j" % "py4j" % "0.10.7" % "provided",
+ "org.json4s" % "json4s-native_2.12" % "3.7.0-M11" % "provided",
Review Comment:
This plug is build separately and not being in the assembly. Do I still need to move these to `project/Dependencies.scala` ?
--
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@toree.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Jupyter Spark monitor [incubator-toree]
Posted by "lresende (via GitHub)" <gi...@apache.org>.
lresende merged PR #229:
URL: https://github.com/apache/incubator-toree/pull/229
--
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@toree.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org