Re: Review Request 39906: SQOOP-2652 Sqoop2: Add test case for S3 incremental import to HDFS

[Abraham Fine]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39906/#review105785
---

Ship it!


Ship It!

- Abraham Fine


On Nov. 9, 2015, 11:28 p.m., Jarek Cecho wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39906/
> ---
> 
> (Updated Nov. 9, 2015, 11:28 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2652
> https://issues.apache.org/jira/browse/SQOOP-2652
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> Added test case as suggeted on JIRA.
> 
> 
> Diffs
> -
> 
>   test/src/main/java/org/apache/sqoop/test/testcases/JettyTestCase.java 
> 2349f1c 
>   test/src/test/java/org/apache/sqoop/integration/connector/hdfs/S3Test.java 
> 73a9acf 
> 
> Diff: https://reviews.apache.org/r/39906/diff/
> 
> 
> Testing
> ---
> 
> And the test case is passing!
> 
> mvn clean test -pl test -Dtest=S3Test 
> -Dorg.apache.sqoop.integration.connector.hdfs.s3.bucket=sqoop 
> -Dorg.apache.sqoop.integration.connector.hdfs.s3.access=AK... 
> -Dorg.apache.sqoop.integration.connector.hdfs.s3.secret=93...
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>

Re: Review Request 39906: SQOOP-2652 Sqoop2: Add test case for S3 incremental import to HDFS

[Abraham Fine]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39906/#review105701
---



test/src/main/java/org/apache/sqoop/test/testcases/JettyTestCase.java (line 247)


the names confuse me and there is a little bit of duplicated code here. 
maybe we could do something like:

```
protected void createFromFile(String filename, String... lines) throws 
IOException {
  createFromFileWithHdfsClient(hdfsClent, filename, lines);
}

protected void createFromFileWithHdfsClient(FileSystem hdfsClient, 
filename, String... lines) throws IOException {
  
HdfsUtils.createFile(hdfsClient,HdfsUtils.joinPathFragments(getMapreduceDirectory(),
 filename), lines);
}
```



test/src/test/java/org/apache/sqoop/integration/connector/hdfs/S3Test.java 
(line 73)


will maven attempt to run this test if i do:
mvn integration-test -pl test ?

given the external dependency on s3, it may be better to prevent this from 
runinng by default


- Abraham Fine


On Nov. 4, 2015, 7:25 p.m., Jarek Cecho wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39906/
> ---
> 
> (Updated Nov. 4, 2015, 7:25 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2652
> https://issues.apache.org/jira/browse/SQOOP-2652
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> Added test case as suggeted on JIRA.
> 
> 
> Diffs
> -
> 
>   test/src/main/java/org/apache/sqoop/test/testcases/JettyTestCase.java 
> 2349f1c 
>   test/src/test/java/org/apache/sqoop/integration/connector/hdfs/S3Test.java 
> 73a9acf 
> 
> Diff: https://reviews.apache.org/r/39906/diff/
> 
> 
> Testing
> ---
> 
> And the test case is passing!
> 
> mvn clean test -pl test -Dtest=S3Test 
> -Dorg.apache.sqoop.integration.connector.hdfs.s3.bucket=sqoop 
> -Dorg.apache.sqoop.integration.connector.hdfs.s3.access=AK... 
> -Dorg.apache.sqoop.integration.connector.hdfs.s3.secret=93...
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>

Re: Review Request 39906: SQOOP-2652 Sqoop2: Add test case for S3 incremental import to HDFS

[Jarek Cecho]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39906/
---

(Updated Nov. 9, 2015, 11:28 p.m.)


Review request for Sqoop.


Changes
---

Incorporating Abe's comments.


Bugs: SQOOP-2652
https://issues.apache.org/jira/browse/SQOOP-2652


Repository: sqoop-sqoop2


Description
---

Added test case as suggeted on JIRA.


Diffs (updated)
-

  test/src/main/java/org/apache/sqoop/test/testcases/JettyTestCase.java 2349f1c 
  test/src/test/java/org/apache/sqoop/integration/connector/hdfs/S3Test.java 
73a9acf 

Diff: https://reviews.apache.org/r/39906/diff/


Testing
---

And the test case is passing!

mvn clean test -pl test -Dtest=S3Test 
-Dorg.apache.sqoop.integration.connector.hdfs.s3.bucket=sqoop 
-Dorg.apache.sqoop.integration.connector.hdfs.s3.access=AK... 
-Dorg.apache.sqoop.integration.connector.hdfs.s3.secret=93...


Thanks,

Jarek Cecho

Re: Review Request 39906: SQOOP-2652 Sqoop2: Add test case for S3 incremental import to HDFS

[Jarek Cecho]

> On Nov. 9, 2015, 5:40 p.m., Abraham Fine wrote:
> > test/src/test/java/org/apache/sqoop/integration/connector/hdfs/S3Test.java, 
> > line 75
> > 
> >
> > will maven attempt to run this test if i do:
> > mvn integration-test -pl test ?
> > 
> > given the external dependency on s3, it may be better to prevent this 
> > from runinng by default

Good question - maven will try to run it, but the code will throw 
"SkipException" if S3 is not available which will make maven correctly skip the 
test.

In the future I would like to refactore it more to a groups (e.g. all tests 
that have s3 dependency), but that is probably outside of scope of this JIRA.


> On Nov. 9, 2015, 5:40 p.m., Abraham Fine wrote:
> > test/src/main/java/org/apache/sqoop/test/testcases/JettyTestCase.java, line 
> > 247
> > 
> >
> > the names confuse me and there is a little bit of duplicated code here. 
> > maybe we could do something like:
> > 
> > ```
> > protected void createFromFile(String filename, String... lines) throws 
> > IOException {
> >   createFromFileWithHdfsClient(hdfsClent, filename, lines);
> > }
> > 
> > protected void createFromFileWithHdfsClient(FileSystem hdfsClient, 
> > filename, String... lines) throws IOException {
> >   
> > HdfsUtils.createFile(hdfsClient,HdfsUtils.joinPathFragments(getMapreduceDirectory(),
> >  filename), lines);
> > }
> > ```

Fair point, will do.


- Jarek


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39906/#review105701
---


On Nov. 4, 2015, 7:25 p.m., Jarek Cecho wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39906/
> ---
> 
> (Updated Nov. 4, 2015, 7:25 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2652
> https://issues.apache.org/jira/browse/SQOOP-2652
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> Added test case as suggeted on JIRA.
> 
> 
> Diffs
> -
> 
>   test/src/main/java/org/apache/sqoop/test/testcases/JettyTestCase.java 
> 2349f1c 
>   test/src/test/java/org/apache/sqoop/integration/connector/hdfs/S3Test.java 
> 73a9acf 
> 
> Diff: https://reviews.apache.org/r/39906/diff/
> 
> 
> Testing
> ---
> 
> And the test case is passing!
> 
> mvn clean test -pl test -Dtest=S3Test 
> -Dorg.apache.sqoop.integration.connector.hdfs.s3.bucket=sqoop 
> -Dorg.apache.sqoop.integration.connector.hdfs.s3.access=AK... 
> -Dorg.apache.sqoop.integration.connector.hdfs.s3.secret=93...
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>

Re: Review Request 39906: SQOOP-2652 Sqoop2: Add test case for S3 incremental import to HDFS

[Jarek Cecho]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39906/
---

(Updated Nov. 4, 2015, 7:25 p.m.)


Review request for Sqoop.


Changes
---

Made sure that the test case won't cause side effect of skipping all tests in 
given suite.


Bugs: SQOOP-2652
https://issues.apache.org/jira/browse/SQOOP-2652


Repository: sqoop-sqoop2


Description
---

Added test case as suggeted on JIRA.


Diffs (updated)
-

  test/src/main/java/org/apache/sqoop/test/testcases/JettyTestCase.java 2349f1c 
  test/src/test/java/org/apache/sqoop/integration/connector/hdfs/S3Test.java 
73a9acf 

Diff: https://reviews.apache.org/r/39906/diff/


Testing
---

And the test case is passing!

mvn clean test -pl test -Dtest=S3Test 
-Dorg.apache.sqoop.integration.connector.hdfs.s3.bucket=sqoop 
-Dorg.apache.sqoop.integration.connector.hdfs.s3.access=AK... 
-Dorg.apache.sqoop.integration.connector.hdfs.s3.secret=93...


Thanks,

Jarek Cecho

Review Request 39906: SQOOP-2652 Sqoop2: Add test case for S3 incremental import to HDFS

[Jarek Cecho]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39906/
---

Review request for Sqoop.


Bugs: SQOOP-2652
https://issues.apache.org/jira/browse/SQOOP-2652


Repository: sqoop-sqoop2


Description
---

Added test case as suggeted on JIRA.


Diffs
-

  test/src/main/java/org/apache/sqoop/test/testcases/JettyTestCase.java d967aaa 
  test/src/test/java/org/apache/sqoop/integration/connector/hdfs/S3Test.java 
73a9acf 

Diff: https://reviews.apache.org/r/39906/diff/


Testing
---

And the test case is passing!

mvn clean test -pl test -Dtest=S3Test 
-Dorg.apache.sqoop.integration.connector.hdfs.s3.bucket=sqoop 
-Dorg.apache.sqoop.integration.connector.hdfs.s3.access=AK... 
-Dorg.apache.sqoop.integration.connector.hdfs.s3.secret=93...


Thanks,

Jarek Cecho

Re: Review Request 39906: SQOOP-2652 Sqoop2: Add test case for S3 incremental import to HDFS

[Jarek Cecho]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39906/
---

(Updated Nov. 3, 2015, 9:58 p.m.)


Review request for Sqoop.


Changes
---

Fixing case when the S3 is not properly defined.


Bugs: SQOOP-2652
https://issues.apache.org/jira/browse/SQOOP-2652


Repository: sqoop-sqoop2


Description
---

Added test case as suggeted on JIRA.


Diffs (updated)
-

  test/src/main/java/org/apache/sqoop/test/testcases/JettyTestCase.java d967aaa 
  test/src/test/java/org/apache/sqoop/integration/connector/hdfs/S3Test.java 
73a9acf 

Diff: https://reviews.apache.org/r/39906/diff/


Testing
---

And the test case is passing!

mvn clean test -pl test -Dtest=S3Test 
-Dorg.apache.sqoop.integration.connector.hdfs.s3.bucket=sqoop 
-Dorg.apache.sqoop.integration.connector.hdfs.s3.access=AK... 
-Dorg.apache.sqoop.integration.connector.hdfs.s3.secret=93...


Thanks,

Jarek Cecho