[
https://issues.apache.org/jira/browse/HADOOP-8531?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13404528#comment-13404528
]
Harsh J commented on HADOOP-8531:
---------------------------------
Hi,
Thanks for the patch. Functionally it looks good. However, please address these
style and message nits that needs to be followed to ensure uniform code look
across Hadoop code :)
In general we follow Sun's Code style conventions, with 2 spaces for
indentation.
{code}if(this.keySerializer == null) {{code}
Please place a space after {{if}}.
{code}"Not able to get serializer for value class
"+valClass.getCanonicalName(){code}
Please place a space on either sides of {{+}}. Additionally, in this message
and in other messages such as this, it would be better to borrow the style I've
used at MAPREDUCE-2584, that also prompts the user to check their core-site.xml
configuration properties to load proper classes to make this operation
successful.
{code}fs,conf, path,{code}
Please space out the commas properly between {{fs}} and {{conf}}, as done with
the other params. Same applies to other places you would notice this in your
patch.
{code}fail("should throw IOException");{code}
The failing message can also be more clear on _why_ an IOException is expected
here (i.e. purpose, being missing serializers or deserializers).
Can you also test if deserializer-fetching needs the same work and edit the
patch appropriately?
Thanks again! :)
P.s. Please do keep in mind the code conventions when submitting patches as it
helps with speedier acceptance :)
> SequenceFile Writer can throw out a better error if a serializer isn't
> available
> --------------------------------------------------------------------------------
>
> Key: HADOOP-8531
> URL: https://issues.apache.org/jira/browse/HADOOP-8531
> Project: Hadoop Common
> Issue Type: Improvement
> Reporter: Harsh J
> Assignee: madhukara phatak
> Priority: Trivial
> Labels: newbie
> Attachments: HADOOP-8531.patch
>
>
> Currently, if the provided Key/Value class lacks a proper serializer in the
> loaded config for the SequenceFile.Writer, we get an NPE as the null return
> goes unchecked.
> Hence we get:
> {code}
> java.lang.NullPointerException
> at org.apache.hadoop.io.SequenceFile$Writer.init(SequenceFile.java:1163)
> at
> org.apache.hadoop.io.SequenceFile$Writer.<init>(SequenceFile.java:1079)
> at
> org.apache.hadoop.io.SequenceFile$RecordCompressWriter.<init>(SequenceFile.java:1331)
> at org.apache.hadoop.io.SequenceFile.createWriter(SequenceFile.java:271)
> {code}
> We can provide a better message + exception in such cases. This is slightly
> related to MAPREDUCE-2584.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
