[GitHub] [spark] Ngone51 edited a comment on pull request #29788: [SPARK-32913][CORE][K8S] Improve ExecutorDecommissionInfo and ExecutorDecommissionState for different use cases

[GitBox <gi...@apache.org>]

Ngone51 edited a comment on pull request #29788:
URL: https://github.com/apache/spark/pull/29788#issuecomment-696475960


   > Since there was another PR in the same area committed that broke the existing integration tests in this area I don't feel confident with my soft reservations and switching to a vetoing for this change (e.g. -1).
   
   So it turns out that the PR (https://github.com/apache/spark/pull/29722) doesn't break the existing integration tests. (It's someone else but we don't know yet). Therefore, I think we are safe to continue the discussion.
   
   So as for the main concern, I think I can actually change the `ExecutorDecommissionInfo` to:
   
   `ExecutorDecommissionInfo(message, hostOpt, isDynamic, isTriggeredByExecutor)`
   
   Then, we'd still keep one decommission info instance. Does this sounds good to you?
   
   
   (BTW, the updates could be late since the PR (https://github.com/apache/spark/pull/29722) is already reverted and we have conflits here. And the re-submitted PR is being block by the broken integration tests.)


----------------------------------------------------------------
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.

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