AngersZhuuuu commented on a change in pull request #34710:
URL: https://github.com/apache/spark/pull/34710#discussion_r757188225
##########
File path:
resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala
##########
@@ -181,7 +180,7 @@ private[spark] class Client(
// Get a new application from our RM
val newApp = yarnClient.createApplication()
val newAppResponse = newApp.getNewApplicationResponse()
- appId = newAppResponse.getApplicationId()
+ this.appId = newAppResponse.getApplicationId()
Review comment:
> Is the idea that setting it here means its new value is visible in the
catch clauses below?
There are two reason to do this:
1. It's a refactor that we don't need to define each `appId` since we can
use this one `this.appId`. In current code it define tree `appId`, not
necessary.
2. We can just use this appId for both `yarn-client`, `yarn-cluster` mode.
Since when I build some internal plugin, when yarn-client mode here the value
is null but cluster mode it have the right value.
--
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
