ivoson commented on code in PR #37268:
URL: https://github.com/apache/spark/pull/37268#discussion_r947947321


##########
core/src/main/scala/org/apache/spark/resource/ResourceProfileBuilder.scala:
##########
@@ -76,6 +79,17 @@ class ResourceProfileBuilder() {
     this
   }
 
+  /**
+   * Specify target resource profile type.
+   * @param isTaskResourceProfile will create a [[TaskResourceProfile]] if 
true, otherwise a
+   *                              normal [[ResourceProfile]]
+   * @return This ResourceProfileBuilder
+   */
+  def taskOnly(isTaskResourceProfile: Boolean = true): this.type = {

Review Comment:
   > Please don't force push the code, it makes reviewing what changed much 
harder.
   Thanks. Will avoid force push in future.
   
   > but that likely still becomes an issue with dynamic allocation support.
   Yes, agree. My previous concern is that we try to limit 
`TaskResourceProfile` to dynamic allocation off, exception will be thrown  if 
user build a `TaskResourceProfile` when dynamic allocation enabled. This will 
introduce behavior change.
   If we don't have the limit, then we need to think about the policy for 
scheduling tasks with `TaskResourceProfile` when dynamic allocation is enabled. 
 
   
   cc @Ngone51 @tgravescs What do you think? Shall we support dynamic 
allocation as well 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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to