Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/8093#discussion_r36819662
--- Diff:
yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
@@ -423,6 +424,7 @@ private[yarn] class YarnAllocator(
for (completedContainer <- completedContainers) {
val containerId = completedContainer.getContainerId
val alreadyReleased = releasedContainers.remove(containerId)
+ var completedContainerReason : String = ""
--- End diff --
this is kinda subjective, but its often nice to use a `val` which gets
assigned as the result of the conditionals. That way its clear that the value
is fixed at the end. The downside is that sometimes it makes the assignment a
bit harder to follow eg. if there is a lot of nesting. this case is kinda
borderline IMO. in any case, the code would look like:
```scala
val completedContainerReason: Option[String] = if (!alreadyReleased) {
...
if (completedContainer.getExitStatus = ContainerExistStatus.PREEMPTED) {
...
Some(s"Container preempted: $containerId") // this is the "result" of
the if
} else if ... {
...
}
} else {
None
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
