> On Feb. 4, 2015, 5:57 p.m., Gwen Shapira wrote: > > common/src/main/java/org/apache/sqoop/model/Input.java, lines 62-66 > > <https://reviews.apache.org/r/30537/diff/5/?file=846604#file846604line62> > > > > I think this should be a List of Input and not a String with commas. > > This will ensure that we depend on Inputs that actually exist and will > > be easier to use in the code (where we actually implement the overrides) > > later. > > Veena Basavaraj wrote: > it is just names of inputs at this point, we validate that the input > names exist etc in the code. > > from String to List<MInput> ? and how is it expressed in the actual > config class. > > Veena Basavaraj wrote: > Would just be even easier if you can provide the sample code here. > > Gwen Shapira wrote: > Yep, I saw that. So why keep it as string and then convert to > List<MInput> later? Any reason not to start with the list? > We are not limited to primitives here, for examples, we use a List of > Validators, so I can't see why not List of Inputs...
following discussion with Veena: the syntax for list of inputs is verbose, and we expect very short lists that will be converted to actual lists early on (and that only refers to inputs defined in the same Config class) So, ok with leaving this as is. - Gwen ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30537/#review70993 ----------------------------------------------------------- On Feb. 3, 2015, 10:01 p.m., Veena Basavaraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30537/ > ----------------------------------------------------------- > > (Updated Feb. 3, 2015, 10:01 p.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-1804 > https://issues.apache.org/jira/browse/SQOOP-1804 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > see JIRA > > derby updates > for input attributes > and storing input relations > > > Diffs > ----- > > common/src/main/java/org/apache/sqoop/error/code/CommonRepositoryError.java > 952be3f > common/src/main/java/org/apache/sqoop/json/util/ConfigInputConstants.java > 5d261de > > common/src/main/java/org/apache/sqoop/json/util/ConfigInputSerialization.java > 8b11ed5 > common/src/main/java/org/apache/sqoop/model/ConfigUtils.java 279f3a6 > common/src/main/java/org/apache/sqoop/model/Input.java b6305f2 > common/src/main/java/org/apache/sqoop/model/InputEditable.java PRE-CREATION > common/src/main/java/org/apache/sqoop/model/MBooleanInput.java c7fcf90 > common/src/main/java/org/apache/sqoop/model/MConfig.java d128441 > common/src/main/java/org/apache/sqoop/model/MEnumInput.java 74838fc > common/src/main/java/org/apache/sqoop/model/MInput.java d1c2ab6 > common/src/main/java/org/apache/sqoop/model/MIntegerInput.java de50cd4 > common/src/main/java/org/apache/sqoop/model/MMapInput.java cf781b2 > common/src/main/java/org/apache/sqoop/model/MStringInput.java 4854838 > common/src/test/java/org/apache/sqoop/json/util/ConfigTestUtil.java 7f0e2f1 > > common/src/test/java/org/apache/sqoop/json/util/TestConfigSerialization.java > 10ac3cf > common/src/test/java/org/apache/sqoop/model/TestConfigUtils.java 64ffdd1 > common/src/test/java/org/apache/sqoop/model/TestMAccountableEntity.java > c0644e7 > common/src/test/java/org/apache/sqoop/model/TestMBooleanInput.java 57d2da2 > common/src/test/java/org/apache/sqoop/model/TestMConfig.java 908348d > common/src/test/java/org/apache/sqoop/model/TestMConfigList.java 4f8261c > common/src/test/java/org/apache/sqoop/model/TestMConnector.java e1c02aa > common/src/test/java/org/apache/sqoop/model/TestMEnumInput.java 39f09ce > common/src/test/java/org/apache/sqoop/model/TestMIntegerInput.java 69e511f > common/src/test/java/org/apache/sqoop/model/TestMJob.java 605f429 > common/src/test/java/org/apache/sqoop/model/TestMLink.java e985c17 > common/src/test/java/org/apache/sqoop/model/TestMMapInput.java c9b786b > common/src/test/java/org/apache/sqoop/model/TestMNamedElement.java f851cbd > common/src/test/java/org/apache/sqoop/model/TestMStringInput.java a4faf95 > common/src/test/java/org/apache/sqoop/model/TestMValidatedElement.java > f0bdda4 > > connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestGenericJdbcConnectorUpgrader.java > 1b1fa7f > core/src/test/java/org/apache/sqoop/driver/TestDriverConfigUpgrader.java > d4522ae > > repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java > 4feaee6 > > repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java > 8626b31 > > repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositorySchemaConstants.java > 4ab07b2 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java > 2f05fcb > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaConstants.java > a551094 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaCreateQuery.java > ad80797 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaUpgradeQuery.java > fa6710b > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java > be8c23e > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestConnectorHandling.java > ca24398 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestDriverHandling.java > e12bf46 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestInputTypes.java > fb07152 > shell/src/main/java/org/apache/sqoop/shell/core/Constants.java ca387d8 > shell/src/main/java/org/apache/sqoop/shell/utils/ConfigDisplayer.java > e240163 > shell/src/main/java/org/apache/sqoop/shell/utils/ConfigFiller.java de40818 > shell/src/main/resources/shell-resource.properties 2b5a9b7 > > Diff: https://reviews.apache.org/r/30537/diff/ > > > Testing > ------- > > yes > > > Thanks, > > Veena Basavaraj > >
