[
https://issues.apache.org/jira/browse/HDFS-6430?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14030159#comment-14030159
]
Mike Yoder commented on HDFS-6430:
----------------------------------
I'm new to both Hadoop and Java, so please take my comments and questions with
that perspective. :-)
Overall looks good; comments below are mostly just nits!
Looks like there are some whitespace issues going on:
{noformat}
$ git apply HDFS-6430.2.patch
/mnt/shared/HDFS-6430.2.patch:80: trailing whitespace.
DELETE(HTTP_DELETE), SETXATTR(HTTP_PUT), GETXATTRS(HTTP_GET),
/mnt/shared/HDFS-6430.2.patch:91: trailing whitespace.
/mnt/shared/HDFS-6430.2.patch:105: trailing whitespace.
* @return HttpURLConnection a <code>HttpURLConnection</code> for the
/mnt/shared/HDFS-6430.2.patch:106: trailing whitespace.
* HttpFSServer server, authenticated and ready to use for the
/mnt/shared/HDFS-6430.2.patch:112: trailing whitespace.
Map<String, String> params, Map<String, List<String>> multiValuedParams,
warning: squelched 67 whitespace errors
warning: 72 lines add whitespace errors.
{noformat}
HttpFSFileSystem.java
- lines 1129 and 1146 - a foreach() style loop might be marginally more clear
- Please add javadoc for new functions
HttpFSUtils.java
- OK
FSOperations.java
- line 259 - could probably also be replaced by foreach()
HttpFSParametersProvider.java
- line 487 - should this be a constant somewhere?
HttpFSServer.java
- lines 341, 349, 568, 577 - don't you need some note about the operation
that's being performed in the audit message?
Parameters.java
ParametersProvider.java
- I confess that the type magic here makes my head hurt, so I'm going to punt
on this one.
BaseTestHttpFSWith.java
- I like the tests.
- Would be nice if you could also test different formats for values other than
hex
- Some negative tests - like invalid param/value would be nice (I guess I
should have done this for the acl tests...)
PBHelper.java
- OK
EnumSetParam.java
- parse() - isn't the code in the for loop just a split(",")? If not, how not?
- toString() - wouldn't a foreach() be easier here, too?
Thanks for doing this; I was actually going to pick up this issue myself until
I saw you had it. [~tucu00] is also going to have a look.
> HTTPFS - Implement XAttr support
> --------------------------------
>
> Key: HDFS-6430
> URL: https://issues.apache.org/jira/browse/HDFS-6430
> Project: Hadoop HDFS
> Issue Type: Task
> Affects Versions: 3.0.0
> Reporter: Yi Liu
> Assignee: Yi Liu
> Fix For: 3.0.0
>
> Attachments: HDFS-6430.1.patch, HDFS-6430.2.patch, HDFS-6430.patch
>
>
> Add xattr support to HttpFS.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
