----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27881/#review60935 -----------------------------------------------------------
common/src/main/java/org/apache/sqoop/json/util/SchemaSerialization.java <https://reviews.apache.org/r/27881/#comment102352> The `Initializer` will return a `NullSchema` instance now, if no specific schema is provided. So null as schema will violate the design. As other methods do not have defensive check against null, is the check for schema necessary? common/src/main/java/org/apache/sqoop/json/util/SchemaSerialization.java <https://reviews.apache.org/r/27881/#comment102353> Add one blank line please common/src/main/java/org/apache/sqoop/json/util/SchemaSerialization.java <https://reviews.apache.org/r/27881/#comment102356> Please keep setter chain in multiple lines common/src/main/java/org/apache/sqoop/schema/NullSchema.java <https://reviews.apache.org/r/27881/#comment102357> I hate to add complexity, but `NullSchema` should be immutable, right? All setter methods should be hide. Maybe singleton implementation as well? - Qian Xu On Nov. 12, 2014, 8:45 a.m., Veena Basavaraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27881/ > ----------------------------------------------------------- > > (Updated Nov. 12, 2014, 8:45 a.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-1621 > https://issues.apache.org/jira/browse/SQOOP-1621 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > see jira > > The point of this ticket is to guard the connector developers. > > Internally in Sqoop we can still create a empty Schema object and male sure > we dont have to worry about guarding everyy single place a NPE for null > schema can happen:) > > hence the code change is in the SchemaSerialization class so that we handle > null schema > > > Diffs > ----- > > common/src/main/java/org/apache/sqoop/json/util/SchemaSerialization.java > ecb5aa3 > common/src/main/java/org/apache/sqoop/schema/NullSchema.java PRE-CREATION > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsFromInitializer.java > 4c6f566 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsToInitializer.java > bce72b5 > docs/src/site/sphinx/ConnectorDevelopment.rst 58b3b61 > > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/MRConfigurationUtils.java > d5f74f0 > > execution/mapreduce/src/test/java/org/apache/sqoop/job/mr/TestMRConfigurationUtils.java > fbe3e7b > spi/src/main/java/org/apache/sqoop/job/etl/Initializer.java d66b099 > > Diff: https://reviews.apache.org/r/27881/diff/ > > > Testing > ------- > > yes ( unit and integration ) > > > Thanks, > > Veena Basavaraj > >
