> On Oct. 26, 2014, 3:39 p.m., Gwen Shapira wrote: > > Hate to contradict Veena, but I don't think this is ready yet. For one > > thing, the integration tests this patch touches are failing on my machine, > > and the validator looks broken as well.
hehe, I did not run the patch, I proabably should have. I am glad you did not have to agree:) > On Oct. 26, 2014, 3:39 p.m., Gwen Shapira wrote: > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfig.java, > > line 34 > > <https://reviews.apache.org/r/27110/diff/3/?file=733151#file733151line34> > > > > I love the idea of verifying the URI, but something looks wrong here. > > > > 1. The regex assumes two ":" are mandatory, i.e. we'll always have a > > port number. But hdfs:///tmp and hdfs://ns1/tmp are both valid. > > 2. It looks like we only allow a single character between the ":"? > > i.e. hdfs://a:8920 matches but hdfs://abcd:8020 does not. This is obviously > > broken. > > > > So, please add unit tests for this. very good catch. > On Oct. 26, 2014, 3:39 p.m., Gwen Shapira wrote: > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfig.java, > > lines 41-43 > > <https://reviews.apache.org/r/27110/diff/3/?file=733151#file733151line41> > > > > Users will love it if we give an example for a legal URI in the error > > message. i.e: "... for example the URI hdfs://nameservice1:8020/sqoop is > > valid". It gives them something to copy paste. love this comment too Gwen! I love making anything that makes users life happy. - Veena ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27110/#review58574 ----------------------------------------------------------- On Oct. 24, 2014, 5:12 p.m., Abraham Elmahrek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27110/ > ----------------------------------------------------------- > > (Updated Oct. 24, 2014, 5:12 p.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-1620 > https://issues.apache.org/jira/browse/SQOOP-1620 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > commit ef7c1e58b115fec6c6d5a84345ca1e3c0cc93164 > Author: Abraham Elmahrek <[email protected]> > Date: Fri Oct 10 19:02:27 2014 -0700 > > SQOOP-1620: Sqoop2: FileSystem should be configurable in HDFS connector > > :100644 100644 cce0e29... 7ad66f8... M > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java > :100644 100644 31b0a99... 0a2c98b... M > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java > :100644 100644 c7d35f7... 6d79db7... M > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsFromDestroyer.java > :100644 100644 0752510... 4c6f566... M > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsFromInitializer.java > :100644 100644 682349c... 0753f9d... M > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java > :100644 100644 daa7fe2... 12bcd53... M > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsPartitioner.java > :100644 100644 8bfd727... 3c85be8... M > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsToDestroyer.java > :100644 100644 e3d54b8... bce72b5... M > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsToInitializer.java > :000000 100644 0000000... 0d77427... A > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsUtils.java > :000000 100644 0000000... 54768ea... A > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfig.java > :000000 100644 0000000... 29063a8... A > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfiguration.java > :100644 100644 90bc8bc... 3d088d0... M > connector/connector-hdfs/src/main/resources/hdfs-connector-config.properties > :100644 100644 124c3df... 0a6369f... M > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java > :000000 100644 0000000... 846b565... A > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestHdfsUtils.java > :100644 100644 b1b3b16... f88424b... M > test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/FromHDFSToRDBMSTest.java > :100644 100644 36f7443... b89424f... M > test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/FromRDBMSToHDFSTest.java > > > Diffs > ----- > > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java > cce0e29 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java > 31b0a99 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsFromDestroyer.java > c7d35f7 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsFromInitializer.java > 0752510 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java > 682349c > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsPartitioner.java > daa7fe2 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsToDestroyer.java > 8bfd727 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsToInitializer.java > e3d54b8 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsUtils.java > PRE-CREATION > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfig.java > PRE-CREATION > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfiguration.java > PRE-CREATION > > connector/connector-hdfs/src/main/resources/hdfs-connector-config.properties > 90bc8bc > > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java > 124c3df > > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestHdfsUtils.java > PRE-CREATION > > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java > 8429e15 > > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestPartitioner.java > bef1984 > > test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/FromHDFSToRDBMSTest.java > b1b3b16 > > test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/FromRDBMSToHDFSTest.java > 36f7443 > > Diff: https://reviews.apache.org/r/27110/diff/ > > > Testing > ------- > > mvn test and manually tested working order. > > > Thanks, > > Abraham Elmahrek > >
