sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Abraham Fine" <abef...@cloudera.com>
Subject Re: Review Request 40895: SQOOP-2720
Date Mon, 07 Dec 2015 18:43:12 GMT


> On Dec. 5, 2015, 2:28 p.m., Jarek Cecho wrote:
> > test/src/test/java/org/apache/sqoop/integration/connector/kafka/FromHDFSToKafkaTest.java,
line 26
> > <https://reviews.apache.org/r/40895/diff/4/?file=1153632#file1153632line26>
> >
> >     Why not run this test on real cluster with real Kafka instance?

that is something that we can definitely do, but i figured that it was beyond the scope of
this jira and something we should tackle at a later date.


> On Dec. 5, 2015, 2:28 p.m., Jarek Cecho wrote:
> > test/src/test/java/org/apache/sqoop/integration/connector/kafka/FromRDBMSToKafkaTest.java,
line 28
> > <https://reviews.apache.org/r/40895/diff/4/?file=1153633#file1153633line28>
> >
> >     Why not run this test on real cluster with real Kafka instance?

see other comment on kafka test


> On Dec. 5, 2015, 2:28 p.m., Jarek Cecho wrote:
> > test/src/main/java/org/apache/sqoop/test/minicluster/RealSqoopCluster.java, lines
50-68
> > <https://reviews.apache.org/r/40895/diff/4/?file=1153629#file1153629line50>
> >
> >     Can we create a util class that for given SqoopClient will drop all jobs/links?
> >     
> >     It seems that we're having this code sniplet a lot, so it would be great to
reuse the code - happy to review this as part of separate JIRA if needed.

whoops, forgot to clean that up :)


> On Dec. 5, 2015, 2:28 p.m., Jarek Cecho wrote:
> > pom.xml, lines 233-259
> > <https://reviews.apache.org/r/40895/diff/4/?file=1153628#file1153628line233>
> >
> >     I'm not a big fan of the current approach of using maven profiles for all the
various groups we have - it's hard to mantain and get oriented in. Would it make sense to
simply document how to specify the given groups on command line or is that too hard to do?

i am sure that is something that maven supports. it just seemed to me that this seemed like
a perfect use case for a profile. i think we only have two groups at the moment so i can not
really see this getting out of hand any time soon.


> On Dec. 5, 2015, 2:28 p.m., Jarek Cecho wrote:
> > test/src/main/java/org/apache/sqoop/test/testcases/JettyTestCase.java, lines 107-111
> > <https://reviews.apache.org/r/40895/diff/4/?file=1153630#file1153630line107>
> >
> >     Let's keep the "setMethodName" and create new BeforeMethod call to drop the
getMapreduceDirectory()?

my concern here is that getMapreduceDirectory is dependent on having the name set. so, considering
these two methods seem to always be called together, i would rather not create the opportunity
to have a bug in beforemethod order.


> On Dec. 5, 2015, 2:28 p.m., Jarek Cecho wrote:
> > test/src/main/java/org/apache/sqoop/test/testcases/JettyTestCase.java, lines 166-168
> > <https://reviews.apache.org/r/40895/diff/4/?file=1153630#file1153630line166>
> >
> >     I'm generally +1 on migrating to the MiniClusterFactory to be honest, but let's
make that as standalone patch - very likely a subtask of SQOOP-2329.

so we already use the SqoopMiniClusterFactory in the SqoopInfrastructureProvider. I really
do not see an easy way to run the JettyTestCase tests on a real cluster without it. is this
change really worthy of its own jira?


- Abraham


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


On Dec. 4, 2015, 7:46 a.m., Abraham Fine wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40895/
> -----------------------------------------------------------
> 
> (Updated Dec. 4, 2015, 7:46 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2720
>     https://issues.apache.org/jira/browse/SQOOP-2720
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> Sqoop2: Allow the integration tests to be run against a real cluster
> 
> 
> Diffs
> -----
> 
>   pom.xml 91721ce 
>   test/src/main/java/org/apache/sqoop/test/minicluster/RealSqoopCluster.java 325a6a9

>   test/src/main/java/org/apache/sqoop/test/testcases/JettyTestCase.java bd4ba6a 
>   test/src/test/java/org/apache/sqoop/integration/connector/hive/FromRDBMSToKiteHiveTest.java
0e46bf3 
>   test/src/test/java/org/apache/sqoop/integration/connector/kafka/FromHDFSToKafkaTest.java
aa062fb 
>   test/src/test/java/org/apache/sqoop/integration/connector/kafka/FromRDBMSToKafkaTest.java
6e78a13 
>   test/src/test/java/org/apache/sqoop/integration/connectorloading/BlacklistedConnectorTest.java
6228b0d 
>   test/src/test/java/org/apache/sqoop/integration/connectorloading/ClasspathTest.java
4a2e7a4 
>   test/src/test/java/org/apache/sqoop/integration/repository/derby/upgrade/Derby1_99_3UpgradeTest.java
8d02e24 
>   test/src/test/java/org/apache/sqoop/integration/repository/derby/upgrade/Derby1_99_4UpgradeTest.java
b88940a 
>   test/src/test/java/org/apache/sqoop/integration/repository/derby/upgrade/Derby1_99_5UpgradeTest.java
1f3563d 
>   test/src/test/java/org/apache/sqoop/integration/repository/derby/upgrade/Derby1_99_6UpgradeTest.java
a57a420 
>   test/src/test/java/org/apache/sqoop/integration/server/ShowJobInOrderTest.java cbf1e90

> 
> Diff: https://reviews.apache.org/r/40895/diff/
> 
> 
> Testing
> -------
> 
> yes
> 
> 
> Thanks,
> 
> Abraham Fine
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message