[
https://issues.apache.org/jira/browse/HADOOP-14394?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16024067#comment-16024067
]
Andrew Wang commented on HADOOP-14394:
--------------------------------------
Thanks for working on this Eddy! Looks good overall, some code review comments:
* Recommend we rename to just {{createFile}} rather than {{createFileBuilder}},
simpler
* Should make sure there's javadoc for all public methods of the StreamBuilder,
even if they're just links to look at other javadoc (e.g. the setOption
overloads)
* Why did we take the defaults out of the getters and put them into the
constructor? This increases coupling with the parent class.
* I found a stack overflow that explains how to reuse the parent builder
without doing all the overrides:
https://stackoverflow.com/questions/17164375/subclassing-a-java-builder-class .
Need to combine the answers from gkamal and Stephan Vavra.
* Rather than splitting out {{validate}} methods, I think we can also remove
the client-side validation, since the NN should also be doing this same
validation anyway. Should keep/add tests for these though.
* The {{create}} in NameNodeConnector is unnecessary, since
{{createFileBuilder}} already sets this flag.
* TestDistributedFileSystem, please don't use a wildcard import
setOptions comments
* I'd recommend splitting this out into its own change. Steve wanted setOptions
for all the FS-specific functionality, and this patch doesn't do that.
* I see you chose to name the new options like HDFS config keys and put then in
HdfsClientConfigKeys. I think this is confusing, since these aren't config keys
in the classical sense. IMO these should go in a different public class, and
with simpler names (e.g. "dfs.no-local-write"). Would appreciate Steve's input
on this.
* We should exercise all the setOption methods at least in a test
> Provide Builder pattern for DistributedFileSystem.create
> --------------------------------------------------------
>
> Key: HADOOP-14394
> URL: https://issues.apache.org/jira/browse/HADOOP-14394
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs
> Affects Versions: 2.9.0
> Reporter: Lei (Eddy) Xu
> Assignee: Lei (Eddy) Xu
> Attachments: HADOOP-14394.00.patch, HADOOP-14394.01.patch
>
>
> This JIRA continues to refine the {{FSOutputStreamBuilder}} interface
> introduced in HDFS-11170.
> It should also provide a spec for the Builder API.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
