[ 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