You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/02/12 07:12:34 UTC
[GitHub] [spark] Yikun opened a new pull request #35496: [SPARK-37145][K8S][FOLLOWUP] Add note for KubernetesDriverCustomFeatureConfigStep
Yikun opened a new pull request #35496:
URL: https://github.com/apache/spark/pull/35496
### What changes were proposed in this pull request?
Add note for developer to show how to use `KubernetesDriverCustomFeatureConfigStep` and `KubernetesExecutorCustomFeatureConfigStep`.
### Why are the changes needed?
Give an example to show how to use it.
### Does this PR introduce _any_ user-facing change?
No, doc only
### How was this patch tested?
ci 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@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun closed pull request #35496: [SPARK-37145][K8S][FOLLOWUP] Add note for `KubernetesCustom[Driver/Executor]FeatureConfigStep`
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #35496:
URL: https://github.com/apache/spark/pull/35496
--
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@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #35496: [SPARK-37145][K8S][FOLLOWUP] Add note for KubernetesDriverCustomFeatureConfigStep
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #35496:
URL: https://github.com/apache/spark/pull/35496#issuecomment-1038110328
Looking fine but let me cc @dongjoon-hyun @attilapiros FYI
--
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@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #35496: [SPARK-37145][K8S][FOLLOWUP] Add note for `KubernetesCustom[Driver/Executor]FeatureConfigStep`
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #35496:
URL: https://github.com/apache/spark/pull/35496#discussion_r805462704
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesDriverCustomFeatureConfigStep.scala
##########
@@ -25,6 +25,45 @@ import org.apache.spark.deploy.k8s.KubernetesDriverConf
* A base interface to help user extend custom feature step in driver side.
* Note: If your custom feature step would be used only in driver or both in driver and executor,
* please use this.
+ *
+ * Example of driver feature step:
+ *
+ * {{{
+ * class DriverExampleFeatureStep extends KubernetesDriverCustomFeatureConfigStep {
+ * private var driverConf: KubernetesDriverConf = _
+ *
+ * override def init(conf: KubernetesDriverConf): Unit = {
+ * driverConf = conf
+ * }
+ *
+ * // Implements methods of `KubernetesFeatureConfigStep`, such as `configurePod`
+ * override def configurePod(pod: SparkPod): SparkPod = {
+ * // Apply modifications on the given pod in accordance to this feature.
+ * }
+ * }
+ * }}}
+ *
+ * Example of feature step for both driver and executor:
+ *
+ * {{{
+ * class ExecutorExampleFeatureStep extends KubernetesDriverCustomFeatureConfigStep
+ * with KubernetesExecutorCustomFeatureConfigStep {
Review comment:
Sorry for nit-picking but please check [indentation](https://github.com/databricks/scala-style-guide#spacing-and-indentation).
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesExecutorCustomFeatureConfigStep.scala
##########
@@ -25,6 +25,45 @@ import org.apache.spark.deploy.k8s.KubernetesExecutorConf
* A base interface to help user extend custom feature step in executor side.
* Note: If your custom feature step would be used only in driver or both in driver and executor,
* please use this.
+ *
+ * Example of executor feature step:
+ *
+ * {{{
+ * class ExecutorExampleFeatureStep extends KubernetesExecutorCustomFeatureConfigStep {
+ * private var executorConf: KubernetesExecutorConf = _
+ *
+ * override def init(conf: KubernetesExecutorConf): Unit = {
+ * executorConf = conf
+ * }
+ *
+ * // Implements methods of `KubernetesFeatureConfigStep`, such as `configurePod`
+ * override def configurePod(pod: SparkPod): SparkPod = {
+ * // Apply modifications on the given pod in accordance to this feature.
+ * }
+ * }
+ * }}}
+ *
+ * Example of feature step for both driver and executor:
+ *
+ * {{{
+ * class ExecutorExampleFeatureStep extends KubernetesDriverCustomFeatureConfigStep
+ * with KubernetesExecutorCustomFeatureConfigStep {
Review comment:
ditto
--
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@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #35496: [SPARK-37145][K8S][FOLLOWUP] Add note for `KubernetesCustom[Driver/Executor]FeatureConfigStep`
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35496:
URL: https://github.com/apache/spark/pull/35496#issuecomment-1039428392
Thanks. That looks better, @Yikun .
--
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@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on pull request #35496: [SPARK-37145][K8S][FOLLOWUP] Add note for `KubernetesCustom[Driver/Executor]FeatureConfigStep`
Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #35496:
URL: https://github.com/apache/spark/pull/35496#issuecomment-1038561113
> I'm not sure about the advertising of this ambiguous example naming ExecutorExampleFeatureStep. Since this is a document, we had better choose a better guideline for the name which will serve for both driver and executor.
Maybe `DriverAndExecutorExampleFeatureStep` ?
--
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@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #35496: [SPARK-37145][K8S][FOLLOWUP] Add note for `KubernetesCustom[Driver/Executor]FeatureConfigStep`
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35496:
URL: https://github.com/apache/spark/pull/35496#issuecomment-1039428392
Thanks. That looks better, @Yikun .
--
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@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #35496: [SPARK-37145][K8S][FOLLOWUP] Add note for `KubernetesCustom[Driver/Executor]FeatureConfigStep`
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #35496:
URL: https://github.com/apache/spark/pull/35496#discussion_r805462704
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesDriverCustomFeatureConfigStep.scala
##########
@@ -25,6 +25,45 @@ import org.apache.spark.deploy.k8s.KubernetesDriverConf
* A base interface to help user extend custom feature step in driver side.
* Note: If your custom feature step would be used only in driver or both in driver and executor,
* please use this.
+ *
+ * Example of driver feature step:
+ *
+ * {{{
+ * class DriverExampleFeatureStep extends KubernetesDriverCustomFeatureConfigStep {
+ * private var driverConf: KubernetesDriverConf = _
+ *
+ * override def init(conf: KubernetesDriverConf): Unit = {
+ * driverConf = conf
+ * }
+ *
+ * // Implements methods of `KubernetesFeatureConfigStep`, such as `configurePod`
+ * override def configurePod(pod: SparkPod): SparkPod = {
+ * // Apply modifications on the given pod in accordance to this feature.
+ * }
+ * }
+ * }}}
+ *
+ * Example of feature step for both driver and executor:
+ *
+ * {{{
+ * class ExecutorExampleFeatureStep extends KubernetesDriverCustomFeatureConfigStep
+ * with KubernetesExecutorCustomFeatureConfigStep {
Review comment:
Sorry for nit-picking but please check [indentation](https://github.com/databricks/scala-style-guide#spacing-and-indentation).
--
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@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #35496: [SPARK-37145][K8S][FOLLOWUP] Add note for `KubernetesCustom[Driver/Executor]FeatureConfigStep`
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35496:
URL: https://github.com/apache/spark/pull/35496#issuecomment-1038552808
Thank you for pinging me, @HyukjinKwon .
--
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@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] attilapiros commented on a change in pull request #35496: [SPARK-37145][K8S][FOLLOWUP] Add note for KubernetesDriverCustomFeatureConfigStep
Posted by GitBox <gi...@apache.org>.
attilapiros commented on a change in pull request #35496:
URL: https://github.com/apache/spark/pull/35496#discussion_r805343474
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesExecutorCustomFeatureConfigStep.scala
##########
@@ -25,6 +25,45 @@ import org.apache.spark.deploy.k8s.KubernetesExecutorConf
* A base interface to help user extend custom feature step in executor side.
* Note: If your custom feature step would be used only in driver or both in driver and executor,
* please use this.
+ *
+ * Example of feature step for both driver and executor:
Review comment:
```suggestion
* Example of executor feature step:
```
--
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@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] attilapiros commented on a change in pull request #35496: [SPARK-37145][K8S][FOLLOWUP] Add note for KubernetesDriverCustomFeatureConfigStep
Posted by GitBox <gi...@apache.org>.
attilapiros commented on a change in pull request #35496:
URL: https://github.com/apache/spark/pull/35496#discussion_r805343474
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesExecutorCustomFeatureConfigStep.scala
##########
@@ -25,6 +25,45 @@ import org.apache.spark.deploy.k8s.KubernetesExecutorConf
* A base interface to help user extend custom feature step in executor side.
* Note: If your custom feature step would be used only in driver or both in driver and executor,
* please use this.
+ *
+ * Example of feature step for both driver and executor:
Review comment:
```suggestion
* Example of executor feature step:
```
--
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@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #35496: [SPARK-37145][K8S][FOLLOWUP] Add note for `KubernetesCustom[Driver/Executor]FeatureConfigStep`
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35496:
URL: https://github.com/apache/spark/pull/35496#issuecomment-1038552808
Thank you for pinging me, @HyukjinKwon .
--
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@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #35496: [SPARK-37145][K8S][FOLLOWUP] Add note for `KubernetesCustom[Driver/Executor]FeatureConfigStep`
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #35496:
URL: https://github.com/apache/spark/pull/35496#discussion_r805462951
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesExecutorCustomFeatureConfigStep.scala
##########
@@ -25,6 +25,45 @@ import org.apache.spark.deploy.k8s.KubernetesExecutorConf
* A base interface to help user extend custom feature step in executor side.
* Note: If your custom feature step would be used only in driver or both in driver and executor,
* please use this.
+ *
+ * Example of executor feature step:
+ *
+ * {{{
+ * class ExecutorExampleFeatureStep extends KubernetesExecutorCustomFeatureConfigStep {
+ * private var executorConf: KubernetesExecutorConf = _
+ *
+ * override def init(conf: KubernetesExecutorConf): Unit = {
+ * executorConf = conf
+ * }
+ *
+ * // Implements methods of `KubernetesFeatureConfigStep`, such as `configurePod`
+ * override def configurePod(pod: SparkPod): SparkPod = {
+ * // Apply modifications on the given pod in accordance to this feature.
+ * }
+ * }
+ * }}}
+ *
+ * Example of feature step for both driver and executor:
+ *
+ * {{{
+ * class ExecutorExampleFeatureStep extends KubernetesDriverCustomFeatureConfigStep
+ * with KubernetesExecutorCustomFeatureConfigStep {
Review comment:
ditto
--
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@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #35496: [SPARK-37145][K8S][FOLLOWUP] Add note for KubernetesDriverCustomFeatureConfigStep
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #35496:
URL: https://github.com/apache/spark/pull/35496#discussion_r805153748
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesDriverCustomFeatureConfigStep.scala
##########
@@ -25,6 +25,39 @@ import org.apache.spark.deploy.k8s.KubernetesDriverConf
* A base interface to help user extend custom feature step in driver side.
* Note: If your custom feature step would be used only in driver or both in driver and executor,
* please use this.
+ *
+ * Here is an example of driver feature step:
Review comment:
```suggestion
* Example of driver feature step:
```
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesDriverCustomFeatureConfigStep.scala
##########
@@ -25,6 +25,39 @@ import org.apache.spark.deploy.k8s.KubernetesDriverConf
* A base interface to help user extend custom feature step in driver side.
* Note: If your custom feature step would be used only in driver or both in driver and executor,
* please use this.
+ *
+ * Here is an example of driver feature step:
+ *
+ * {{{
+ * class DriverExampleFeatureStep extends KubernetesDriverCustomFeatureConfigStep {
+ * private var driverConf: KubernetesDriverConf = _
+ *
+ * override def init(conf: KubernetesDriverConf): Unit = {
+ * driverConf = conf
+ * }
+ *
+ * override def configurePod(pod: SparkPod): SparkPod = {}
Review comment:
Can we add some comments within the block to explain what kind of codes are expected? e.g.)
```scala
override def configurePod(pod: SparkPod): SparkPod = {
// blah blah
}
```
Or probably it should be best to show an actual working example for a scenario.
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesDriverCustomFeatureConfigStep.scala
##########
@@ -25,6 +25,39 @@ import org.apache.spark.deploy.k8s.KubernetesDriverConf
* A base interface to help user extend custom feature step in driver side.
* Note: If your custom feature step would be used only in driver or both in driver and executor,
* please use this.
+ *
+ * Here is an example of driver feature step:
+ *
+ * {{{
+ * class DriverExampleFeatureStep extends KubernetesDriverCustomFeatureConfigStep {
+ * private var driverConf: KubernetesDriverConf = _
+ *
+ * override def init(conf: KubernetesDriverConf): Unit = {
+ * driverConf = conf
+ * }
+ *
+ * override def configurePod(pod: SparkPod): SparkPod = {}
+ * }
+ * }}}
+ *
+ * Here is an example of feature step both for driver and executor:
Review comment:
```suggestion
* Example of feature step for both driver and executor:
```
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesDriverCustomFeatureConfigStep.scala
##########
@@ -25,6 +25,39 @@ import org.apache.spark.deploy.k8s.KubernetesDriverConf
* A base interface to help user extend custom feature step in driver side.
* Note: If your custom feature step would be used only in driver or both in driver and executor,
* please use this.
+ *
+ * Here is an example of driver feature step:
+ *
+ * {{{
+ * class DriverExampleFeatureStep extends KubernetesDriverCustomFeatureConfigStep {
+ * private var driverConf: KubernetesDriverConf = _
+ *
+ * override def init(conf: KubernetesDriverConf): Unit = {
+ * driverConf = conf
+ * }
+ *
+ * override def configurePod(pod: SparkPod): SparkPod = {}
+ * }
+ * }}}
+ *
+ * Here is an example of feature step both for driver and executor:
+ *
+ * {{{
+ * class ExecutorExampleFeatureStep extends KubernetesDriverCustomFeatureConfigStep
+ * with KubernetesExecutorCustomFeatureConfigStep {
Review comment:
```suggestion
* with KubernetesExecutorCustomFeatureConfigStep {
```
--
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@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on pull request #35496: [SPARK-37145][K8S][FOLLOWUP] Add note for `KubernetesCustom[Driver/Executor]FeatureConfigStep`
Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #35496:
URL: https://github.com/apache/spark/pull/35496#issuecomment-1040955479
@dongjoon-hyun addressed! : )
--
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@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #35496: [SPARK-37145][K8S][FOLLOWUP] Add note for KubernetesDriverCustomFeatureConfigStep
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #35496:
URL: https://github.com/apache/spark/pull/35496#issuecomment-1038110328
Looking fine but let me cc @dongjoon-hyun @attilapiros FYI
--
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@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #35496: [SPARK-37145][K8S][FOLLOWUP] Add note for KubernetesDriverCustomFeatureConfigStep
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #35496:
URL: https://github.com/apache/spark/pull/35496#discussion_r805153748
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesDriverCustomFeatureConfigStep.scala
##########
@@ -25,6 +25,39 @@ import org.apache.spark.deploy.k8s.KubernetesDriverConf
* A base interface to help user extend custom feature step in driver side.
* Note: If your custom feature step would be used only in driver or both in driver and executor,
* please use this.
+ *
+ * Here is an example of driver feature step:
Review comment:
```suggestion
* Example of driver feature step:
```
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesDriverCustomFeatureConfigStep.scala
##########
@@ -25,6 +25,39 @@ import org.apache.spark.deploy.k8s.KubernetesDriverConf
* A base interface to help user extend custom feature step in driver side.
* Note: If your custom feature step would be used only in driver or both in driver and executor,
* please use this.
+ *
+ * Here is an example of driver feature step:
+ *
+ * {{{
+ * class DriverExampleFeatureStep extends KubernetesDriverCustomFeatureConfigStep {
+ * private var driverConf: KubernetesDriverConf = _
+ *
+ * override def init(conf: KubernetesDriverConf): Unit = {
+ * driverConf = conf
+ * }
+ *
+ * override def configurePod(pod: SparkPod): SparkPod = {}
Review comment:
Can we add some comments within the block to explain what kind of codes are expected? e.g.)
```scala
override def configurePod(pod: SparkPod): SparkPod = {
// blah blah
}
```
Or probably it should be best to show an actual working example for a scenario.
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesDriverCustomFeatureConfigStep.scala
##########
@@ -25,6 +25,39 @@ import org.apache.spark.deploy.k8s.KubernetesDriverConf
* A base interface to help user extend custom feature step in driver side.
* Note: If your custom feature step would be used only in driver or both in driver and executor,
* please use this.
+ *
+ * Here is an example of driver feature step:
+ *
+ * {{{
+ * class DriverExampleFeatureStep extends KubernetesDriverCustomFeatureConfigStep {
+ * private var driverConf: KubernetesDriverConf = _
+ *
+ * override def init(conf: KubernetesDriverConf): Unit = {
+ * driverConf = conf
+ * }
+ *
+ * override def configurePod(pod: SparkPod): SparkPod = {}
+ * }
+ * }}}
+ *
+ * Here is an example of feature step both for driver and executor:
Review comment:
```suggestion
* Example of feature step for both driver and executor:
```
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesDriverCustomFeatureConfigStep.scala
##########
@@ -25,6 +25,39 @@ import org.apache.spark.deploy.k8s.KubernetesDriverConf
* A base interface to help user extend custom feature step in driver side.
* Note: If your custom feature step would be used only in driver or both in driver and executor,
* please use this.
+ *
+ * Here is an example of driver feature step:
+ *
+ * {{{
+ * class DriverExampleFeatureStep extends KubernetesDriverCustomFeatureConfigStep {
+ * private var driverConf: KubernetesDriverConf = _
+ *
+ * override def init(conf: KubernetesDriverConf): Unit = {
+ * driverConf = conf
+ * }
+ *
+ * override def configurePod(pod: SparkPod): SparkPod = {}
+ * }
+ * }}}
+ *
+ * Here is an example of feature step both for driver and executor:
+ *
+ * {{{
+ * class ExecutorExampleFeatureStep extends KubernetesDriverCustomFeatureConfigStep
+ * with KubernetesExecutorCustomFeatureConfigStep {
Review comment:
```suggestion
* with KubernetesExecutorCustomFeatureConfigStep {
```
--
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@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a change in pull request #35496: [SPARK-37145][K8S][FOLLOWUP] Add note for KubernetesDriverCustomFeatureConfigStep
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35496:
URL: https://github.com/apache/spark/pull/35496#discussion_r805237606
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesDriverCustomFeatureConfigStep.scala
##########
@@ -25,6 +25,39 @@ import org.apache.spark.deploy.k8s.KubernetesDriverConf
* A base interface to help user extend custom feature step in driver side.
* Note: If your custom feature step would be used only in driver or both in driver and executor,
* please use this.
+ *
+ * Here is an example of driver feature step:
+ *
+ * {{{
+ * class DriverExampleFeatureStep extends KubernetesDriverCustomFeatureConfigStep {
+ * private var driverConf: KubernetesDriverConf = _
+ *
+ * override def init(conf: KubernetesDriverConf): Unit = {
+ * driverConf = conf
+ * }
+ *
+ * override def configurePod(pod: SparkPod): SparkPod = {}
Review comment:
```scala
// Implements methods of `KubernetesFeatureConfigStep`, such as `configurePod`
override def configurePod(pod: SparkPod): SparkPod = {
// Apply modifications on the given pod in accordance to this feature.
}
```
How about this? This doc is more for giving a exmaple to introduce how to initialize according input configuration.
--
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@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a change in pull request #35496: [SPARK-37145][K8S][FOLLOWUP] Add note for KubernetesDriverCustomFeatureConfigStep
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35496:
URL: https://github.com/apache/spark/pull/35496#discussion_r805237606
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesDriverCustomFeatureConfigStep.scala
##########
@@ -25,6 +25,39 @@ import org.apache.spark.deploy.k8s.KubernetesDriverConf
* A base interface to help user extend custom feature step in driver side.
* Note: If your custom feature step would be used only in driver or both in driver and executor,
* please use this.
+ *
+ * Here is an example of driver feature step:
+ *
+ * {{{
+ * class DriverExampleFeatureStep extends KubernetesDriverCustomFeatureConfigStep {
+ * private var driverConf: KubernetesDriverConf = _
+ *
+ * override def init(conf: KubernetesDriverConf): Unit = {
+ * driverConf = conf
+ * }
+ *
+ * override def configurePod(pod: SparkPod): SparkPod = {}
Review comment:
```scala
// Implements the methods of `KubernetesFeatureConfigStep`, such as `configurePod`
override def configurePod(pod: SparkPod): SparkPod = {
// Apply modifications on the given pod in accordance to this feature.
}
```
How about this? This doc is more for giving a exmaple to introduce how to initialize according input configuration.
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesDriverCustomFeatureConfigStep.scala
##########
@@ -25,6 +25,39 @@ import org.apache.spark.deploy.k8s.KubernetesDriverConf
* A base interface to help user extend custom feature step in driver side.
* Note: If your custom feature step would be used only in driver or both in driver and executor,
* please use this.
+ *
+ * Here is an example of driver feature step:
+ *
+ * {{{
+ * class DriverExampleFeatureStep extends KubernetesDriverCustomFeatureConfigStep {
+ * private var driverConf: KubernetesDriverConf = _
+ *
+ * override def init(conf: KubernetesDriverConf): Unit = {
+ * driverConf = conf
+ * }
+ *
+ * override def configurePod(pod: SparkPod): SparkPod = {}
Review comment:
```scala
// Implements methods of `KubernetesFeatureConfigStep`, such as `configurePod`
override def configurePod(pod: SparkPod): SparkPod = {
// Apply modifications on the given pod in accordance to this feature.
}
```
How about this? This doc is more for giving a exmaple to introduce how to initialize according input configuration.
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesExecutorCustomFeatureConfigStep.scala
##########
@@ -25,6 +25,45 @@ import org.apache.spark.deploy.k8s.KubernetesExecutorConf
* A base interface to help user extend custom feature step in executor side.
* Note: If your custom feature step would be used only in driver or both in driver and executor,
* please use this.
+ *
+ * Example of feature step for both driver and executor:
Review comment:
oops, addressed
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesDriverCustomFeatureConfigStep.scala
##########
@@ -25,6 +25,45 @@ import org.apache.spark.deploy.k8s.KubernetesDriverConf
* A base interface to help user extend custom feature step in driver side.
* Note: If your custom feature step would be used only in driver or both in driver and executor,
* please use this.
+ *
+ * Example of driver feature step:
+ *
+ * {{{
+ * class DriverExampleFeatureStep extends KubernetesDriverCustomFeatureConfigStep {
+ * private var driverConf: KubernetesDriverConf = _
+ *
+ * override def init(conf: KubernetesDriverConf): Unit = {
+ * driverConf = conf
+ * }
+ *
+ * // Implements methods of `KubernetesFeatureConfigStep`, such as `configurePod`
+ * override def configurePod(pod: SparkPod): SparkPod = {
+ * // Apply modifications on the given pod in accordance to this feature.
+ * }
+ * }
+ * }}}
+ *
+ * Example of feature step for both driver and executor:
+ *
+ * {{{
+ * class ExecutorExampleFeatureStep extends KubernetesDriverCustomFeatureConfigStep
+ * with KubernetesExecutorCustomFeatureConfigStep {
Review comment:
ah, yes.
> For classes whose header doesn't fit in two lines, use 4 space indentation for its parameters, put each in each line, **put the extends on the next line with 2 space indent**, and add a blank line after class header.
This fix was required by https://github.com/apache/spark/pull/35496#discussion_r805153828 , but I think 2 space is right. will recover soon.
--
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@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a change in pull request #35496: [SPARK-37145][K8S][FOLLOWUP] Add note for KubernetesDriverCustomFeatureConfigStep
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35496:
URL: https://github.com/apache/spark/pull/35496#discussion_r805237606
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesDriverCustomFeatureConfigStep.scala
##########
@@ -25,6 +25,39 @@ import org.apache.spark.deploy.k8s.KubernetesDriverConf
* A base interface to help user extend custom feature step in driver side.
* Note: If your custom feature step would be used only in driver or both in driver and executor,
* please use this.
+ *
+ * Here is an example of driver feature step:
+ *
+ * {{{
+ * class DriverExampleFeatureStep extends KubernetesDriverCustomFeatureConfigStep {
+ * private var driverConf: KubernetesDriverConf = _
+ *
+ * override def init(conf: KubernetesDriverConf): Unit = {
+ * driverConf = conf
+ * }
+ *
+ * override def configurePod(pod: SparkPod): SparkPod = {}
Review comment:
```scala
// Implements the methods of `KubernetesFeatureConfigStep`, such as `configurePod`
override def configurePod(pod: SparkPod): SparkPod = {
// Apply modifications on the given pod in accordance to this feature.
}
```
How about this? This doc is more for giving a exmaple to introduce how to initialize according input configuration.
--
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@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a change in pull request #35496: [SPARK-37145][K8S][FOLLOWUP] Add note for KubernetesDriverCustomFeatureConfigStep
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35496:
URL: https://github.com/apache/spark/pull/35496#discussion_r805345193
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesExecutorCustomFeatureConfigStep.scala
##########
@@ -25,6 +25,45 @@ import org.apache.spark.deploy.k8s.KubernetesExecutorConf
* A base interface to help user extend custom feature step in executor side.
* Note: If your custom feature step would be used only in driver or both in driver and executor,
* please use this.
+ *
+ * Example of feature step for both driver and executor:
Review comment:
oops, addressed
--
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@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #35496: [SPARK-37145][K8S][FOLLOWUP] Add note for `KubernetesCustom[Driver/Executor]FeatureConfigStep`
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35496:
URL: https://github.com/apache/spark/pull/35496#issuecomment-1040936807
Gentle ping, @Yikun .
--
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@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org