hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Andrew Wang (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-11170) Add create API in filesystem public class to support assign parameter through builder
Date Thu, 23 Mar 2017 01:03:41 GMT

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

Andrew Wang commented on HDFS-11170:
------------------------------------

Hi Sammi, thanks for revving, looks pretty close to me,

* DFS, do we need to clone when setting/getting favored nodes? I think users would be okay
with a shallow copy since builders are almost always short-lived.
* I recommended earlier to embed the default logic into the getters, since it lets you avoid
doing a separate setDefaultValue call. Any reason not to do this?
* Some unused imports and checkstyles to clean up
* Since valid values of {{permission}} and {{flags}} cannot be null, shall we add {{Precondition.checkArgument}}
checks to their setters?

Tests
* I think we can also replace the file contents checks with {{ContractTestUtils#verifyFileContents}}
or similar
* There's no need to catch unexpected exceptions just to rethrow them or fail. TestDFS and
TestLFS do this.

TestDFS:
* typo "an file" -> "a file"
* We could probably include this basic functionality test in an existing testcase to save
a minicluster (which takes a few seconds to start and stop)

This is a minor nit, but part of the joy of a builder is how it can be chained. We can shorten
this in {{TestFavoredNodesEndToEnd}} (and probably elsewhere):

{code}
      DistributedFileSystem.HdfsDataOutputStreamBuilder b =
          dfs.newFSDataOutputStreamBuilder(p);
      b.setFavoredNodes(dns);
      FSDataOutputStream out = b.build();
{code}

to

{code}
      FSDataOutputStream out = dfs.newFSDataOutputStreamBuilder(p);
          .setFavoredNodes(dns)
          .build();
{code}

Since the unit tests are often looked to for example code, it'd be nice to clean these up.

> Add create API in filesystem public class to support assign parameter through builder
> -------------------------------------------------------------------------------------
>
>                 Key: HDFS-11170
>                 URL: https://issues.apache.org/jira/browse/HDFS-11170
>             Project: Hadoop HDFS
>          Issue Type: Improvement
>            Reporter: SammiChen
>            Assignee: Wei Zhou
>              Labels: hdfs-ec-3.0-nice-to-have
>         Attachments: HDFS-11170-00.patch, HDFS-11170-01.patch, HDFS-11170-02.patch, HDFS-11170-03.patch,
HDFS-11170-04.patch, HDFS-11170-05.patch
>
>
> FileSystem class supports multiple create functions to help user create file. Some create
functions has many parameters, it's hard for user to exactly remember these parameters and
their orders. This task is to add builder  based create functions to help user more easily
create file. 



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

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


Mime
View raw message