[ 
https://issues.apache.org/jira/browse/TEZ-373?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13995645#comment-13995645
 ] 

Bikas Saha commented on TEZ-373:
--------------------------------

The patch looks close. I may not have been clear in my original comment. So 
apologies for that. The current patch helps me clarify it.
Instead of {code}+public class TezUserPayload {
+  private final UserPayload payload;
+
+  public TezUserPayload(UserPayload payload) {
+    Preconditions.checkNotNull(payload, "payload is null");
+    this.payload = payload;
+  }
+
+  public UserPayload getProto() {
+    return payload;
+  }
+}{code}

How about we do {code}+public class TezUserPayload {
+  private final byte[] null;
+
+  public TezUserPayload(byte[] userPayload) {
+    Preconditions.checkNotNull(payload, "payload is null");
+    this.payload = payload;
+  }
+
+  public byte[] getPayload() {
+    return payload;
+  }
+}{code}
The creation of a TezUserPayload happens during 1) at the API boundary when 
user passes in the byte[] payload 2) at the RPC boundary when a RPC data is 
converted to objects. TezUserPayload is converted to UserPayload when 
serializing before the outgoing RPC call. These 2 operations can use 
TezUserPayload DAGTypeConverter.convertToTezUserPayload(UserPayload) and 
UserPayload DAGTypeConverter.fromTezUserPayload. Does this make sense? Lets 
discuss if this is cleaner wrt the code.

In this case, IMO we should be returning a TezUserPayload object with empty 
byte array. Because we need to pass around a valid object that will then 
deliver a null payload (since the user did not set a payload)
{code}+  public static TezUserPayload convertTezUserPayloadFromByte(@Nullable 
byte[] payload) {
+    if (payload == null) {
+      return null;
+    }{code}


> Create UserPayload class
> ------------------------
>
>                 Key: TEZ-373
>                 URL: https://issues.apache.org/jira/browse/TEZ-373
>             Project: Apache Tez
>          Issue Type: Sub-task
>            Reporter: Bikas Saha
>            Assignee: Tsuyoshi OZAWA
>              Labels: newbie
>         Attachments: TEZ-373.1.patch, TEZ-373.2.patch, TEZ-373.3.patch, 
> TEZ-373.4.patch, TEZ-373.5.patch, TEZ-373.6.patch, TEZ-373.7.patch, 
> TEZ-373.8.patch
>
>
> Currently Tez allows user payload to be passed as byte[]. Within Tez code is 
> hard to understand where byte[] is userPayload and where is not user payload. 
> If we create a TezUserPayload class that contains byte[] as a member then its 
> much easier to search and identify in code.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to