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

Tsuyoshi OZAWA commented on TEZ-1194:
-------------------------------------

Sorry for delay and thank you for the ping, [~bikassaha]. I'm updating based on 
your comments and I can attach the patch by tomorrow if we can decide following 
points:

Should DagTypeConverter#convert*DAGPlan check the versions?
{code}
  public static InputDescriptor convertInputDescriptorFromDAGPlan(
      TezEntityDescriptorProto proto) {
    String className = proto.getClassName();
    TezUserPayload userPayload = null;
    if (proto.hasUserPayload()) {
      // TODO: Adding version support
      userPayload =  
DagTypeConverters.convertToTezUserPayload(proto.getUserPayload().toByteArray(), 
0);
    }
    return new InputDescriptor(className).setUserPayload(userPayload);
  }
{code}

{quote}
These if stmts should actually become if(userPayload.getPayload() != null) or 
better if(userPayload.hasPayload())
{quote}

I prefer to use {{if(userPayload.hasPayload())}}, and payload in the 
user-facing APIs should be not Nullable. Do you agree with this? If the answer 
is positive, should we do this on another JIRA?

{quote}
In most cases, we should be seeing the DAGTypeConverter code only in near the 
RPC serde code.
{quote}

I agree with this point.


> Make TezUserPayload user facing for payload specification and change to 
> ByteBuffer
> ----------------------------------------------------------------------------------
>
>                 Key: TEZ-1194
>                 URL: https://issues.apache.org/jira/browse/TEZ-1194
>             Project: Apache Tez
>          Issue Type: Sub-task
>            Reporter: Bikas Saha
>            Assignee: Tsuyoshi OZAWA
>            Priority: Blocker
>         Attachments: TEZ-1194.1.patch, TEZ-1194.2.patch
>
>
> Now that we have TezUserPayload being used internally to represent user 
> payload it may be useful to make it user facing on the API for specifying 
> payloads. Advantages
> 1) Clear code for the user instead of having untyped byte[] everywhere
> 2) Lets us internally evolve the representation of user payload and make it 
> more efficient without having to break APIs. We can start with 
> TezUserPayload(byte[]) and then move on to TezUserPayload(ByteBuffer) and so 
> on while maintaining backwards compatibility without needing to add new 
> methods. Old code can be translated within TezUserPayload while user migrates 
> the code.



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

Reply via email to