----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40335/#review106744 -----------------------------------------------------------
common/src/main/java/org/apache/sqoop/json/JSONUtils.java (line 83) <https://reviews.apache.org/r/40335/#comment165503> this null scares me. if i was to glance at this code i would see the containskey check at the top and then assume that we will always return a value. perhaps we should throw an exception when the value for a field is null (a different one than when the value for a field does not exist). - Abraham Fine On Nov. 15, 2015, 11:38 p.m., Jarek Cecho wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40335/ > ----------------------------------------------------------- > > (Updated Nov. 15, 2015, 11:38 p.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-2688 > https://issues.apache.org/jira/browse/SQOOP-2688 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > The patch looks quite huge, but it's really just changing a lot of accessors > to use the safe one. > > > Diffs > ----- > > common/src/main/java/org/apache/sqoop/json/ConnectorBean.java 2509e3e > common/src/main/java/org/apache/sqoop/json/ConnectorsBean.java 88b71f5 > common/src/main/java/org/apache/sqoop/json/DriverBean.java 6f03be2 > common/src/main/java/org/apache/sqoop/json/JSONUtils.java ff8f9ea > common/src/main/java/org/apache/sqoop/json/JobBean.java df7a804 > common/src/main/java/org/apache/sqoop/json/JobsBean.java c62ab49 > common/src/main/java/org/apache/sqoop/json/LinkBean.java 99e7319 > common/src/main/java/org/apache/sqoop/json/LinksBean.java 6e4a906 > common/src/main/java/org/apache/sqoop/json/PrincipalBean.java e540b75 > common/src/main/java/org/apache/sqoop/json/PrincipalsBean.java a4d56dc > common/src/main/java/org/apache/sqoop/json/PrivilegeBean.java 6819063 > common/src/main/java/org/apache/sqoop/json/PrivilegesBean.java fbca54b > common/src/main/java/org/apache/sqoop/json/RoleBean.java e1f5783 > common/src/main/java/org/apache/sqoop/json/RolesBean.java da8fc1f > common/src/main/java/org/apache/sqoop/json/SubmissionBean.java c7d6958 > common/src/main/java/org/apache/sqoop/json/SubmissionsBean.java 019abf1 > common/src/main/java/org/apache/sqoop/json/ValidationResultBean.java > 7dfd9fc > > common/src/main/java/org/apache/sqoop/json/util/ConfigInputSerialization.java > bd4f5c4 > common/src/main/java/org/apache/sqoop/json/util/SchemaSerialization.java > 1fa5743 > common/src/main/java/org/apache/sqoop/json/util/SerializationError.java > d5cdb31 > common/src/test/java/org/apache/sqoop/json/TestJSONUtils.java 67ab412 > > Diff: https://reviews.apache.org/r/40335/diff/ > > > Testing > ------- > > I've run only unit tests, let's see how the precommit hook will like the > patch. > > > Thanks, > > Jarek Cecho > >
