----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31173/#review73095 -----------------------------------------------------------
Seems cool! common/src/main/java/org/apache/sqoop/json/JobConfigInputBean.java <https://reviews.apache.org/r/31173/#comment119301> MConfigType.valueOf? Instead of doing all this string comparison... but this works too. common/src/main/java/org/apache/sqoop/json/JobConfigInputBean.java <https://reviews.apache.org/r/31173/#comment119300> Not sure if calls like this work with SQOOP-1516. FROM_SUB_TYPE doesn't exist? common/src/main/java/org/apache/sqoop/json/JobConfigInputBean.java <https://reviews.apache.org/r/31173/#comment119302> Default case? Or just return an empty object is sufficient? Maybe throw a runtime exception? common/src/main/java/org/apache/sqoop/json/JobConfigInputBean.java <https://reviews.apache.org/r/31173/#comment119303> Default case that throws a runtime exception? Seems like this case should result in the user seeing a 400 error. Also, NPE if this is null... which seems possible right now. server/src/main/java/org/apache/sqoop/handler/ConfigInputRequestHandler.java <https://reviews.apache.org/r/31173/#comment119305> Throw an exception if we don't support the method? server/src/main/java/org/apache/sqoop/handler/ConfigInputRequestHandler.java <https://reviews.apache.org/r/31173/#comment119307> default case? server/src/main/java/org/apache/sqoop/handler/ConfigInputRequestHandler.java <https://reviews.apache.org/r/31173/#comment119306> default case? server/src/main/java/org/apache/sqoop/handler/ConfigInputRequestHandler.java <https://reviews.apache.org/r/31173/#comment119308> provide default cases? server/src/main/java/org/apache/sqoop/handler/ConfigInputRequestHandler.java <https://reviews.apache.org/r/31173/#comment119309> if config is null, JobConfigInputBean functions will throw NPE. server/src/main/webapp/WEB-INF/web.xml <https://reviews.apache.org/r/31173/#comment119297> NIT spacing seems off here? - Abraham Elmahrek On Feb. 19, 2015, 1:15 a.m., Veena Basavaraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31173/ > ----------------------------------------------------------- > > (Updated Feb. 19, 2015, 1:15 a.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-2113 > https://issues.apache.org/jira/browse/SQOOP-2113 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > see jira > > > Diffs > ----- > > common/src/main/java/org/apache/sqoop/error/code/ServerError.java > 86cca7dcc9c7ea0f69b279dd85d3e68a1eda6596 > common/src/main/java/org/apache/sqoop/json/JobBean.java > 0561adebd3b604b27512e22227db076712f27fd0 > common/src/main/java/org/apache/sqoop/json/JobConfigInputBean.java > PRE-CREATION > common/src/main/java/org/apache/sqoop/json/JsonBean.java > 1dd275e80df25b3521bb20dd31e4e2221db50ff8 > common/src/main/java/org/apache/sqoop/json/LinkBean.java > 2e2406f3de35294896e280b57d505197a6d6af86 > common/src/main/java/org/apache/sqoop/json/LinkConfigInputBean.java > PRE-CREATION > > common/src/main/java/org/apache/sqoop/json/util/ConfigInputSerialization.java > ffaf041baed67974b76f3b789b14414ec487a0a4 > common/src/main/java/org/apache/sqoop/model/ConfigUtils.java > dd6e9ce481c12b7d096f4e8473acbdb2f04eee90 > common/src/test/java/org/apache/sqoop/json/TestJobConfigInputBean.java > PRE-CREATION > common/src/test/java/org/apache/sqoop/json/TestLinkConfigInputBean.java > PRE-CREATION > common/src/test/java/org/apache/sqoop/json/util/ConfigTestUtil.java > fbc7faa3a24810952fd22d30b69ee1e983e04547 > > server/src/main/java/org/apache/sqoop/handler/ConfigInputRequestHandler.java > PRE-CREATION > server/src/main/java/org/apache/sqoop/server/v1/ConfigInputServlet.java > PRE-CREATION > server/src/main/webapp/WEB-INF/web.xml > 60ee8c4acace2aaab8909f5c6daa6600875f7794 > > Diff: https://reviews.apache.org/r/31173/diff/ > > > Testing > ------- > > done > > > Thanks, > > Veena Basavaraj > >
