> On Nov. 11, 2014, 7 p.m., Qian Xu wrote: > > common/src/main/java/org/apache/sqoop/json/util/SchemaSerialization.java, > > line 72 > > <https://reviews.apache.org/r/27881/diff/5/?file=758636#file758636line72> > > > > 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? > > Veena Basavaraj wrote: > I am not sure I understand the violation, part, since I changed it to be > a NullSchema class, this check is not required. I will remove it. > > can you clarify what you mean by violation
As a function, it is pretty good practice to not make assumptions on how this function will be called and who calls, AFAI have seen it is good coding practice to be be defensive and not cause embarassing NPE, in java it is pretty verbose if check. other languages are much succint. One of the things I have noticed in all the Beans and its corr extract/ restor method is that it expects every field of that bean to be present. If I have to update a LINK name from a JSON field, and I have to POST a JSON I have to give every field that the extract methods expects, which is such a pain. I really did not have time to create a ticket for each and fix it:) I will someday. Try testing JSON apis with post data, you will see the pain. - Veena ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27881/#review60935 ----------------------------------------------------------- On Nov. 11, 2014, 8:38 p.m., Veena Basavaraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27881/ > ----------------------------------------------------------- > > (Updated Nov. 11, 2014, 8:38 p.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 > > common/src/test/java/org/apache/sqoop/json/util/TestSchemaSerialization.java > 1640ead > > 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 > >
