[
https://issues.apache.org/jira/browse/YARN-2103?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14010438#comment-14010438
]
Tsuyoshi OZAWA commented on YARN-2103:
--------------------------------------
Hi [~decster], thank you for taking this JIRA. I fixed the title of this JIRA.
Please correct me if I misunderstand what you're trying to solve. I have some
comments(it's non-binding one):
1. Can we remove {{builder == null}} check if {{builder}} is initialized with
{{SerializedExceptionProto.newBuilder()}}?
{code}
private void maybeInitBuilder() {
if (viaProto || builder == null) {
builder = SerializedExceptionProto.newBuilder(proto);
}
viaProto = false;
}
{code}
2. Should we use {{else if}} with braces instead of using {{else}} in
{{equals}} like this?
{code}
public boolean equals(Object other) {
if (other == null) {
return false;
} else if (other.getClass().isAssignableFrom(this.getClass())) {
return this.getProto().equals(this.getClass().cast(other).getProto());
}
return false;
}
{code}
3. How about adding concrete tests as a first step of generic tests on
YARN-2051? What do you think, [~djp]?
> Inconsistency between viaProto flag and initial value of
> SerializedExceptionProto.Builder
> -----------------------------------------------------------------------------------------
>
> Key: YARN-2103
> URL: https://issues.apache.org/jira/browse/YARN-2103
> Project: Hadoop YARN
> Issue Type: Bug
> Reporter: Binglin Chang
> Assignee: Binglin Chang
> Attachments: YARN-2103.v1.patch
>
>
> {code}
> SerializedExceptionProto proto = SerializedExceptionProto
> .getDefaultInstance();
> SerializedExceptionProto.Builder builder = null;
> boolean viaProto = false;
> {code}
> Since viaProto is false, we should initiate build rather than proto
--
This message was sent by Atlassian JIRA
(v6.2#6252)
