> On Oct. 20, 2014, 9:14 p.m., Veena Basavaraj wrote: > > connector/connector-kite/src/main/java/org/apache/sqoop/connector/kite/KiteConnector.java, > > line 66 > > <https://reviews.apache.org/r/26963/diff/1/?file=726930#file726930line66> > > > > Instead of exception, we could encourage > > > > We should use the NullConfiguration class in this case, saying this > > connector does not support FROM > > > > just a thought. > > Qian Xu wrote: > Good thought. From the name, I guess NullConfiguration means nothing to > configure. Actually FROM has something to configure. For short-term wait, > throw an exception is IMHO acceptable.
Example of using EmptyConfiguration https://reviews.apache.org/r/27004/, jarcec committed it as well > On Oct. 20, 2014, 9:14 p.m., Veena Basavaraj wrote: > > connector/connector-kite/src/main/java/org/apache/sqoop/connector/kite/KiteConnector.java, > > line 71 > > <https://reviews.apache.org/r/26963/diff/1/?file=726930#file726930line71> > > > > does even default make sense here? We have a enum for direction and it > > is already typed, so why will there ever be the the default case? > > > > public enum Direction { > > FROM, > > TO > > } > > Qian Xu wrote: > `default` here is used to suppress a compiler error, otherwise compiler > will complain about missing a `return` statement. you could retur null as well, this code path should happen in reality. I would not want to throw exceptions when it is not a use case for it > On Oct. 20, 2014, 9:14 p.m., Veena Basavaraj wrote: > > connector/connector-kite/src/main/java/org/apache/sqoop/connector/kite/KiteToInitializer.java, > > line 83 > > <https://reviews.apache.org/r/26963/diff/1/?file=726936#file726936line83> > > > > why cant be we return null ? > > Qian Xu wrote: > Good point. I checked the code, somewhere else (e.g. Matcher) will check > schema with `schema.isEmpty()`. If it returns `null` here, any check schema > methods should be changed to `schema == null || schema.isEmpty()`. > > Veena Basavaraj wrote: > we should guard the matched code, it needs to be defensive, how can we > expect that everyine will return a new Schema, my first gut was to return > null. > > can you file a issue for that? please file a issue for this and assignt o me > On Oct. 20, 2014, 9:14 p.m., Veena Basavaraj wrote: > > connector/connector-kite/src/main/java/org/apache/sqoop/connector/kite/configuration/LinkConfiguration.java, > > line 24 > > <https://reviews.apache.org/r/26963/diff/1/?file=726939#file726939line24> > > > > can we please move thi class to a sdk? and resue it in all connectors? > > > > also nitpick please rename link to linkconfig. > > > > link has a very different meaning in sqoop > > Veena Basavaraj wrote: > please file a ticket for me, I can move it > > Qian Xu wrote: > LinkConfiguration is per connector different. Are you sure you can reuse > them? if the class is exact same field names linkConfig, I am pretty sure we can re use it. if they have different field names, it wont be possible you are right. But in your case it is just one field and you can return the common class in your connector. It should work - Veena ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26963/#review57536 ----------------------------------------------------------- On Oct. 23, 2014, 1:43 a.m., Qian Xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26963/ > ----------------------------------------------------------- > > (Updated Oct. 23, 2014, 1:43 a.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-1588 > https://issues.apache.org/jira/browse/SQOOP-1588 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > Create a basic Kite connector that can write data (i.e. from a jdbc > connection) to HDFS. > > The scope is defined as follows: > * Destination: HDFS > * File Format: Avro Parquet and CSV. > * Compression Codec: Use default > * Partitioner Strategy: Not supported > * Column Mapping: Not supported > > > Diffs > ----- > > connector/connector-kite/pom.xml PRE-CREATION > > connector/connector-kite/src/main/java/org/apache/sqoop/connector/kite/KiteConfigUpgrader.java > PRE-CREATION > > connector/connector-kite/src/main/java/org/apache/sqoop/connector/kite/KiteConnector.java > PRE-CREATION > > connector/connector-kite/src/main/java/org/apache/sqoop/connector/kite/KiteConnectorError.java > PRE-CREATION > > connector/connector-kite/src/main/java/org/apache/sqoop/connector/kite/KiteConstants.java > PRE-CREATION > > connector/connector-kite/src/main/java/org/apache/sqoop/connector/kite/KiteDatasetExecutor.java > PRE-CREATION > > connector/connector-kite/src/main/java/org/apache/sqoop/connector/kite/KiteLoader.java > PRE-CREATION > > connector/connector-kite/src/main/java/org/apache/sqoop/connector/kite/KiteToDestroyer.java > PRE-CREATION > > connector/connector-kite/src/main/java/org/apache/sqoop/connector/kite/KiteToInitializer.java > PRE-CREATION > > connector/connector-kite/src/main/java/org/apache/sqoop/connector/kite/KiteValidator.java > PRE-CREATION > > connector/connector-kite/src/main/java/org/apache/sqoop/connector/kite/configuration/LinkConfig.java > PRE-CREATION > > connector/connector-kite/src/main/java/org/apache/sqoop/connector/kite/configuration/LinkConfiguration.java > PRE-CREATION > > connector/connector-kite/src/main/java/org/apache/sqoop/connector/kite/configuration/ToFormat.java > PRE-CREATION > > connector/connector-kite/src/main/java/org/apache/sqoop/connector/kite/configuration/ToJobConfig.java > PRE-CREATION > > connector/connector-kite/src/main/java/org/apache/sqoop/connector/kite/configuration/ToJobConfiguration.java > PRE-CREATION > > connector/connector-kite/src/main/java/org/apache/sqoop/connector/kite/util/KiteDataTypeUtil.java > PRE-CREATION > connector/connector-kite/src/main/resources/connector-configs.properties > PRE-CREATION > connector/connector-kite/src/main/resources/sqoopconnector.properties > PRE-CREATION > > connector/connector-kite/src/test/java/org/apache/sqoop/connector/kite/TestKiteExecutor.java > PRE-CREATION > > connector/connector-kite/src/test/java/org/apache/sqoop/connector/kite/TestKiteLoader.java > PRE-CREATION > connector/connector-kite/src/test/resources/log4j.properties PRE-CREATION > connector/pom.xml e98a0fc > > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopDestroyerExecutor.java > b385926 > pom.xml f25a29f > server/pom.xml 67baaa5 > test/pom.xml 7a80710 > > Diff: https://reviews.apache.org/r/26963/diff/ > > > Testing > ------- > > New unittests included. All passed. > > > Thanks, > > Qian Xu > >
