hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Aaron Fabbri (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (HADOOP-14396) Add builder interface to FileContext
Date Wed, 12 Jul 2017 22:04:00 GMT

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

Aaron Fabbri edited comment on HADOOP-14396 at 7/12/17 10:03 PM:
-----------------------------------------------------------------

Thanks for the work on this [~eddyxu].  +1 (non-binding) once you clean up the unused import
and get clean Yetus run.

{noformat}
 import org.apache.htrace.core.Tracer;
+import org.apache.zookeeper.Op;
{noformat} 

Accidental import in FileContext.java.


Since I'm new to this Builder stuff, I have a couple comments on the existing design.  We
can create JIRAs to address them if people agree.

{noformat}
+      if (isRecursive()) {
+        createOpts.add(CreateOpts.createParent());
+      } else {
+        createOpts.add(CreateOpts.donotCreateParent());
+      }
{noformat}

- "createAncestors" is a more precise name than "createParent"
- Why not treat absence of "createParent" as an indicator to not do recursive create?  Why
do we need an additional "donotCreateParent" option?  Seems like we have to specify default
behavior anyways.

{noformat}
+    FSDataOutputStream out = fc.create(p).create().build();
+    writeData(fc, p, out, data, data.length);
{noformat}

Can we make {{CreateFlag#CREATE}} the default value, allowing us to omit the extra {{.create()}}
here?




was (Author: fabbri):
Thanks for the work on this [~eddyxu].  +1 (non-binding) once you clean up the unused import.

{noformat}
 import org.apache.htrace.core.Tracer;
+import org.apache.zookeeper.Op;
{noformat} 

Accidental import in FileContext.java.


Since I'm new to this Builder stuff, I have a couple comments on the existing design.  We
can create JIRAs to address them if people agree.

{noformat}
+      if (isRecursive()) {
+        createOpts.add(CreateOpts.createParent());
+      } else {
+        createOpts.add(CreateOpts.donotCreateParent());
+      }
{noformat}

- "createAncestors" is a more precise name than "createParent"
- Why not treat absence of "createParent" as an indicator to not do recursive create?  Why
do we need an additional "donotCreateParent" option?  Seems like we have to specify default
behavior anyways.

{noformat}
+    FSDataOutputStream out = fc.create(p).create().build();
+    writeData(fc, p, out, data, data.length);
{noformat}

Can we make {{CreateFlag#CREATE}} the default value, allowing us to omit the extra {{.create()}}
here?



> Add builder interface to FileContext
> ------------------------------------
>
>                 Key: HADOOP-14396
>                 URL: https://issues.apache.org/jira/browse/HADOOP-14396
>             Project: Hadoop Common
>          Issue Type: Sub-task
>          Components: fs
>    Affects Versions: 2.9.0, 3.0.0-alpha3
>            Reporter: Lei (Eddy) Xu
>            Assignee: Lei (Eddy) Xu
>         Attachments: HADOOP-14396.00.patch
>
>
> Add builder interface for {{FileContext#create}} and {{FileContext#append}}.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


Mime
View raw message