You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Gwen Shapira <gs...@cloudera.com> on 2014/12/08 15:35:49 UTC
Review Request 28802: SQOOP-1852: Sqoop2: Kafka connector supporting
TO direction
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28802/
-----------------------------------------------------------
Review request for Sqoop.
Repository: sqoop-sqoop2
Description
-------
SQOOP-1852: Sqoop2: Kafka connector supporting TO direction
Diffs
-----
connector/connector-kafka/pom.xml PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConnector.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConnectorErrors.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConstants.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaLoader.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaToDestroyer.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaToInitializer.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/JobConfig.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/JobConfiguration.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/LinkConfig.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/LinkConfiguration.java PRE-CREATION
connector/connector-kafka/src/main/resources/kafka-connector-config.properties PRE-CREATION
connector/connector-kafka/src/main/resources/sqoopconnector.properties PRE-CREATION
connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/TestKafkaLoader.java PRE-CREATION
connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/utils/KafkaConsumer.java PRE-CREATION
connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/utils/KafkaLocal.java PRE-CREATION
connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/utils/TestUtil.java PRE-CREATION
connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/utils/ZooKeeperLocal.java PRE-CREATION
connector/connector-kafka/src/test/resources/kafka-server.properties PRE-CREATION
connector/connector-kafka/src/test/resources/log4j.properties PRE-CREATION
connector/pom.xml da4ed3e
pom.xml e182176
Diff: https://reviews.apache.org/r/28802/diff/
Testing
-------
Unit tests only for now.
Thanks,
Gwen Shapira
Re: Review Request 28802: SQOOP-1852: Sqoop2: Kafka connector
supporting TO direction
Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28802/#review64237
-----------------------------------------------------------
Did an initial review... found a couple of small things. Good first go at it!
Also, I still need to try this out on my machine!
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConnectorErrors.java
<https://reviews.apache.org/r/28802/#comment106876>
0000?
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaToDestroyer.java
<https://reviews.apache.org/r/28802/#comment106875>
Log that it's running?
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaToInitializer.java
<https://reviews.apache.org/r/28802/#comment106877>
Log that it ran?
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/JobConfiguration.java
<https://reviews.apache.org/r/28802/#comment106872>
ToJobConfiguration?
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/LinkConfig.java
<https://reviews.apache.org/r/28802/#comment106874>
Add a CSV validator possibly?
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/LinkConfig.java
<https://reviews.apache.org/r/28802/#comment106873>
Comma separated list? Maybe add a CSV validator.
- Abraham Elmahrek
On Dec. 8, 2014, 2:35 p.m., Gwen Shapira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28802/
> -----------------------------------------------------------
>
> (Updated Dec. 8, 2014, 2:35 p.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> SQOOP-1852: Sqoop2: Kafka connector supporting TO direction
>
>
> Diffs
> -----
>
> connector/connector-kafka/pom.xml PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConnector.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConnectorErrors.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConstants.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaLoader.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaToDestroyer.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaToInitializer.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/JobConfig.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/JobConfiguration.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/LinkConfig.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/LinkConfiguration.java PRE-CREATION
> connector/connector-kafka/src/main/resources/kafka-connector-config.properties PRE-CREATION
> connector/connector-kafka/src/main/resources/sqoopconnector.properties PRE-CREATION
> connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/TestKafkaLoader.java PRE-CREATION
> connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/utils/KafkaConsumer.java PRE-CREATION
> connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/utils/KafkaLocal.java PRE-CREATION
> connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/utils/TestUtil.java PRE-CREATION
> connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/utils/ZooKeeperLocal.java PRE-CREATION
> connector/connector-kafka/src/test/resources/kafka-server.properties PRE-CREATION
> connector/connector-kafka/src/test/resources/log4j.properties PRE-CREATION
> connector/pom.xml da4ed3e
> pom.xml e182176
>
> Diff: https://reviews.apache.org/r/28802/diff/
>
>
> Testing
> -------
>
> Unit tests only for now.
>
>
> Thanks,
>
> Gwen Shapira
>
>
Re: Review Request 28802: SQOOP-1852: Sqoop2: Kafka connector
supporting TO direction
Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28802/#review64859
-----------------------------------------------------------
Ship it!
This looks good from my perspective. It fits from a Sqoop connector perspective. I trust you've tested this Gwen.
- Abraham Elmahrek
On Dec. 11, 2014, 8:57 p.m., Gwen Shapira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28802/
> -----------------------------------------------------------
>
> (Updated Dec. 11, 2014, 8:57 p.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> SQOOP-1852: Sqoop2: Kafka connector supporting TO direction
>
>
> Diffs
> -----
>
> common-test/pom.xml 9fd671c
> common-test/src/main/java/org/apache/sqoop/common/test/kafka/KafkaConsumer.java PRE-CREATION
> common-test/src/main/java/org/apache/sqoop/common/test/kafka/KafkaLocal.java PRE-CREATION
> common-test/src/main/java/org/apache/sqoop/common/test/kafka/TestUtil.java PRE-CREATION
> common-test/src/main/java/org/apache/sqoop/common/test/kafka/ZooKeeperLocal.java PRE-CREATION
> connector/connector-kafka/pom.xml PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConnector.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConnectorErrors.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConstants.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaLoader.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaToDestroyer.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaToInitializer.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/LinkConfig.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/LinkConfiguration.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/ToJobConfig.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/ToJobConfiguration.java PRE-CREATION
> connector/connector-kafka/src/main/resources/kafka-connector-config.properties PRE-CREATION
> connector/connector-kafka/src/main/resources/sqoopconnector.properties PRE-CREATION
> connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/TestConfigValidator.java PRE-CREATION
> connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/TestKafkaLoader.java PRE-CREATION
> connector/pom.xml da4ed3e
> pom.xml e182176
> server/pom.xml 1adcca0
> server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java dc237b8
> test/pom.xml eedb545
> test/src/main/java/org/apache/sqoop/test/testcases/KafkaConnectorTestCase.java PRE-CREATION
> test/src/test/java/org/apache/sqoop/integration/connector/kafka/FromRDBMSToKafkaTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/28802/diff/
>
>
> Testing
> -------
>
> Unit tests only for now.
>
>
> Thanks,
>
> Gwen Shapira
>
>
Re: Review Request 28802: SQOOP-1852: Sqoop2: Kafka connector
supporting TO direction
Posted by Gwen Shapira <gs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28802/
-----------------------------------------------------------
(Updated Dec. 11, 2014, 8:57 p.m.)
Review request for Sqoop.
Changes
-------
Added randomized ports for integration tests.
Repository: sqoop-sqoop2
Description
-------
SQOOP-1852: Sqoop2: Kafka connector supporting TO direction
Diffs (updated)
-----
common-test/pom.xml 9fd671c
common-test/src/main/java/org/apache/sqoop/common/test/kafka/KafkaConsumer.java PRE-CREATION
common-test/src/main/java/org/apache/sqoop/common/test/kafka/KafkaLocal.java PRE-CREATION
common-test/src/main/java/org/apache/sqoop/common/test/kafka/TestUtil.java PRE-CREATION
common-test/src/main/java/org/apache/sqoop/common/test/kafka/ZooKeeperLocal.java PRE-CREATION
connector/connector-kafka/pom.xml PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConnector.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConnectorErrors.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConstants.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaLoader.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaToDestroyer.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaToInitializer.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/LinkConfig.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/LinkConfiguration.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/ToJobConfig.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/ToJobConfiguration.java PRE-CREATION
connector/connector-kafka/src/main/resources/kafka-connector-config.properties PRE-CREATION
connector/connector-kafka/src/main/resources/sqoopconnector.properties PRE-CREATION
connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/TestConfigValidator.java PRE-CREATION
connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/TestKafkaLoader.java PRE-CREATION
connector/pom.xml da4ed3e
pom.xml e182176
server/pom.xml 1adcca0
server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java dc237b8
test/pom.xml eedb545
test/src/main/java/org/apache/sqoop/test/testcases/KafkaConnectorTestCase.java PRE-CREATION
test/src/test/java/org/apache/sqoop/integration/connector/kafka/FromRDBMSToKafkaTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/28802/diff/
Testing
-------
Unit tests only for now.
Thanks,
Gwen Shapira
Re: Review Request 28802: SQOOP-1852: Sqoop2: Kafka connector
supporting TO direction
Posted by Gwen Shapira <gs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28802/
-----------------------------------------------------------
(Updated Dec. 11, 2014, 6:20 p.m.)
Review request for Sqoop.
Changes
-------
fixes mentioned in Abe's review
Repository: sqoop-sqoop2
Description
-------
SQOOP-1852: Sqoop2: Kafka connector supporting TO direction
Diffs (updated)
-----
common-test/pom.xml 9fd671c
common-test/src/main/java/org/apache/sqoop/common/test/kafka/KafkaConsumer.java PRE-CREATION
common-test/src/main/java/org/apache/sqoop/common/test/kafka/KafkaLocal.java PRE-CREATION
common-test/src/main/java/org/apache/sqoop/common/test/kafka/TestUtil.java PRE-CREATION
common-test/src/main/java/org/apache/sqoop/common/test/kafka/ZooKeeperLocal.java PRE-CREATION
connector/connector-kafka/pom.xml PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConnector.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConnectorErrors.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConstants.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaLoader.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaToDestroyer.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaToInitializer.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/LinkConfig.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/LinkConfiguration.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/ToJobConfig.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/ToJobConfiguration.java PRE-CREATION
connector/connector-kafka/src/main/resources/kafka-connector-config.properties PRE-CREATION
connector/connector-kafka/src/main/resources/sqoopconnector.properties PRE-CREATION
connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/TestConfigValidator.java PRE-CREATION
connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/TestKafkaLoader.java PRE-CREATION
connector/pom.xml da4ed3e
pom.xml e182176
server/pom.xml 1adcca0
server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java dc237b8
test/pom.xml eedb545
test/src/main/java/org/apache/sqoop/test/testcases/KafkaConnectorTestCase.java PRE-CREATION
test/src/test/java/org/apache/sqoop/integration/connector/kafka/FromRDBMSToKafkaTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/28802/diff/
Testing
-------
Unit tests only for now.
Thanks,
Gwen Shapira
Re: Review Request 28802: SQOOP-1852: Sqoop2: Kafka connector
supporting TO direction
Posted by Gwen Shapira <gs...@cloudera.com>.
> On Dec. 10, 2014, 4:20 a.m., Abraham Elmahrek wrote:
> > common-test/src/main/java/org/apache/sqoop/common/test/kafka/TestUtil.java, line 93
> > <https://reviews.apache.org/r/28802/diff/2/?file=787468#file787468line93>
> >
> > Follow up: randomize?
>
> Gwen Shapira wrote:
> good idea. I'll open a JIRA.
SQOOP-1881
- Gwen
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28802/#review64472
-----------------------------------------------------------
On Dec. 9, 2014, 11:31 p.m., Gwen Shapira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28802/
> -----------------------------------------------------------
>
> (Updated Dec. 9, 2014, 11:31 p.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> SQOOP-1852: Sqoop2: Kafka connector supporting TO direction
>
>
> Diffs
> -----
>
> common-test/pom.xml 9fd671c
> common-test/src/main/java/org/apache/sqoop/common/test/kafka/KafkaConsumer.java PRE-CREATION
> common-test/src/main/java/org/apache/sqoop/common/test/kafka/KafkaLocal.java PRE-CREATION
> common-test/src/main/java/org/apache/sqoop/common/test/kafka/TestUtil.java PRE-CREATION
> common-test/src/main/java/org/apache/sqoop/common/test/kafka/ZooKeeperLocal.java PRE-CREATION
> connector/connector-kafka/pom.xml PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConnector.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConnectorErrors.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConstants.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaLoader.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaToDestroyer.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaToInitializer.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/LinkConfig.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/LinkConfiguration.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/ToJobConfig.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/ToJobConfiguration.java PRE-CREATION
> connector/connector-kafka/src/main/resources/kafka-connector-config.properties PRE-CREATION
> connector/connector-kafka/src/main/resources/sqoopconnector.properties PRE-CREATION
> connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/TestConfigValidator.java PRE-CREATION
> connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/TestKafkaLoader.java PRE-CREATION
> connector/pom.xml da4ed3e
> pom.xml e182176
> server/pom.xml 1adcca0
> server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java dc237b8
> test/pom.xml eedb545
> test/src/main/java/org/apache/sqoop/test/testcases/KafkaConnectorTestCase.java PRE-CREATION
> test/src/test/java/org/apache/sqoop/integration/connector/kafka/FromRDBMSToKafkaTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/28802/diff/
>
>
> Testing
> -------
>
> Unit tests only for now.
>
>
> Thanks,
>
> Gwen Shapira
>
>
Re: Review Request 28802: SQOOP-1852: Sqoop2: Kafka connector
supporting TO direction
Posted by Gwen Shapira <gs...@cloudera.com>.
> On Dec. 10, 2014, 4:20 a.m., Abraham Elmahrek wrote:
> > common-test/src/main/java/org/apache/sqoop/common/test/kafka/KafkaConsumer.java, line 64
> > <https://reviews.apache.org/r/28802/diff/2/?file=787466#file787466line64>
> >
> > 10 seconds or 1 second?
Didn't get this comment. 1000ms = 1s. Does it say 10s anywhere?
> On Dec. 10, 2014, 4:20 a.m., Abraham Elmahrek wrote:
> > common-test/src/main/java/org/apache/sqoop/common/test/kafka/TestUtil.java, line 93
> > <https://reviews.apache.org/r/28802/diff/2/?file=787468#file787468line93>
> >
> > Follow up: randomize?
good idea. I'll open a JIRA.
> On Dec. 10, 2014, 4:20 a.m., Abraham Elmahrek wrote:
> > common-test/src/main/java/org/apache/sqoop/common/test/kafka/ZooKeeperLocal.java, lines 56-60
> > <https://reviews.apache.org/r/28802/diff/2/?file=787469#file787469line56>
> >
> > LOG.error()?
Dropped this since it will generate way too many log messages.
> On Dec. 10, 2014, 4:20 a.m., Abraham Elmahrek wrote:
> > pom.xml, lines 549-551
> > <https://reviews.apache.org/r/28802/diff/2/?file=787486#file787486line549>
> >
> > Is it possible to handle this similar to Hadoop (scope provided)?
Not sure we can assume Kafka jars on Sqoop2 server. Checking for opinions with mailing list.
- Gwen
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28802/#review64472
-----------------------------------------------------------
On Dec. 9, 2014, 11:31 p.m., Gwen Shapira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28802/
> -----------------------------------------------------------
>
> (Updated Dec. 9, 2014, 11:31 p.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> SQOOP-1852: Sqoop2: Kafka connector supporting TO direction
>
>
> Diffs
> -----
>
> common-test/pom.xml 9fd671c
> common-test/src/main/java/org/apache/sqoop/common/test/kafka/KafkaConsumer.java PRE-CREATION
> common-test/src/main/java/org/apache/sqoop/common/test/kafka/KafkaLocal.java PRE-CREATION
> common-test/src/main/java/org/apache/sqoop/common/test/kafka/TestUtil.java PRE-CREATION
> common-test/src/main/java/org/apache/sqoop/common/test/kafka/ZooKeeperLocal.java PRE-CREATION
> connector/connector-kafka/pom.xml PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConnector.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConnectorErrors.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConstants.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaLoader.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaToDestroyer.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaToInitializer.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/LinkConfig.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/LinkConfiguration.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/ToJobConfig.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/ToJobConfiguration.java PRE-CREATION
> connector/connector-kafka/src/main/resources/kafka-connector-config.properties PRE-CREATION
> connector/connector-kafka/src/main/resources/sqoopconnector.properties PRE-CREATION
> connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/TestConfigValidator.java PRE-CREATION
> connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/TestKafkaLoader.java PRE-CREATION
> connector/pom.xml da4ed3e
> pom.xml e182176
> server/pom.xml 1adcca0
> server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java dc237b8
> test/pom.xml eedb545
> test/src/main/java/org/apache/sqoop/test/testcases/KafkaConnectorTestCase.java PRE-CREATION
> test/src/test/java/org/apache/sqoop/integration/connector/kafka/FromRDBMSToKafkaTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/28802/diff/
>
>
> Testing
> -------
>
> Unit tests only for now.
>
>
> Thanks,
>
> Gwen Shapira
>
>
Re: Review Request 28802: SQOOP-1852: Sqoop2: Kafka connector
supporting TO direction
Posted by Gwen Shapira <gs...@cloudera.com>.
> On Dec. 10, 2014, 4:20 a.m., Abraham Elmahrek wrote:
> > common-test/src/main/java/org/apache/sqoop/common/test/kafka/KafkaConsumer.java, line 64
> > <https://reviews.apache.org/r/28802/diff/2/?file=787466#file787466line64>
> >
> > 10 seconds or 1 second?
>
> Gwen Shapira wrote:
> Didn't get this comment. 1000ms = 1s. Does it say 10s anywhere?
Found the "10s" comment and fixing.
- Gwen
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28802/#review64472
-----------------------------------------------------------
On Dec. 9, 2014, 11:31 p.m., Gwen Shapira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28802/
> -----------------------------------------------------------
>
> (Updated Dec. 9, 2014, 11:31 p.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> SQOOP-1852: Sqoop2: Kafka connector supporting TO direction
>
>
> Diffs
> -----
>
> common-test/pom.xml 9fd671c
> common-test/src/main/java/org/apache/sqoop/common/test/kafka/KafkaConsumer.java PRE-CREATION
> common-test/src/main/java/org/apache/sqoop/common/test/kafka/KafkaLocal.java PRE-CREATION
> common-test/src/main/java/org/apache/sqoop/common/test/kafka/TestUtil.java PRE-CREATION
> common-test/src/main/java/org/apache/sqoop/common/test/kafka/ZooKeeperLocal.java PRE-CREATION
> connector/connector-kafka/pom.xml PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConnector.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConnectorErrors.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConstants.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaLoader.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaToDestroyer.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaToInitializer.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/LinkConfig.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/LinkConfiguration.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/ToJobConfig.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/ToJobConfiguration.java PRE-CREATION
> connector/connector-kafka/src/main/resources/kafka-connector-config.properties PRE-CREATION
> connector/connector-kafka/src/main/resources/sqoopconnector.properties PRE-CREATION
> connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/TestConfigValidator.java PRE-CREATION
> connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/TestKafkaLoader.java PRE-CREATION
> connector/pom.xml da4ed3e
> pom.xml e182176
> server/pom.xml 1adcca0
> server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java dc237b8
> test/pom.xml eedb545
> test/src/main/java/org/apache/sqoop/test/testcases/KafkaConnectorTestCase.java PRE-CREATION
> test/src/test/java/org/apache/sqoop/integration/connector/kafka/FromRDBMSToKafkaTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/28802/diff/
>
>
> Testing
> -------
>
> Unit tests only for now.
>
>
> Thanks,
>
> Gwen Shapira
>
>
Re: Review Request 28802: SQOOP-1852: Sqoop2: Kafka connector
supporting TO direction
Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28802/#review64472
-----------------------------------------------------------
Looks good.
There are a few places where System.out.println is used as logging I think. Can we replace it with a logger?
common-test/src/main/java/org/apache/sqoop/common/test/kafka/KafkaConsumer.java
<https://reviews.apache.org/r/28802/#comment107238>
10 seconds or 1 second?
common-test/src/main/java/org/apache/sqoop/common/test/kafka/TestUtil.java
<https://reviews.apache.org/r/28802/#comment107189>
IT seems like zkLocalPort should be here?
common-test/src/main/java/org/apache/sqoop/common/test/kafka/TestUtil.java
<https://reviews.apache.org/r/28802/#comment107239>
Follow up: randomize?
common-test/src/main/java/org/apache/sqoop/common/test/kafka/ZooKeeperLocal.java
<https://reviews.apache.org/r/28802/#comment107190>
LOG.error()?
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaLoader.java
<https://reviews.apache.org/r/28802/#comment107240>
Logging?
pom.xml
<https://reviews.apache.org/r/28802/#comment107193>
Is it possible to handle this similar to Hadoop (scope provided)?
- Abraham Elmahrek
On Dec. 9, 2014, 11:31 p.m., Gwen Shapira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28802/
> -----------------------------------------------------------
>
> (Updated Dec. 9, 2014, 11:31 p.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> SQOOP-1852: Sqoop2: Kafka connector supporting TO direction
>
>
> Diffs
> -----
>
> common-test/pom.xml 9fd671c
> common-test/src/main/java/org/apache/sqoop/common/test/kafka/KafkaConsumer.java PRE-CREATION
> common-test/src/main/java/org/apache/sqoop/common/test/kafka/KafkaLocal.java PRE-CREATION
> common-test/src/main/java/org/apache/sqoop/common/test/kafka/TestUtil.java PRE-CREATION
> common-test/src/main/java/org/apache/sqoop/common/test/kafka/ZooKeeperLocal.java PRE-CREATION
> connector/connector-kafka/pom.xml PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConnector.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConnectorErrors.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConstants.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaLoader.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaToDestroyer.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaToInitializer.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/LinkConfig.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/LinkConfiguration.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/ToJobConfig.java PRE-CREATION
> connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/ToJobConfiguration.java PRE-CREATION
> connector/connector-kafka/src/main/resources/kafka-connector-config.properties PRE-CREATION
> connector/connector-kafka/src/main/resources/sqoopconnector.properties PRE-CREATION
> connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/TestConfigValidator.java PRE-CREATION
> connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/TestKafkaLoader.java PRE-CREATION
> connector/pom.xml da4ed3e
> pom.xml e182176
> server/pom.xml 1adcca0
> server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java dc237b8
> test/pom.xml eedb545
> test/src/main/java/org/apache/sqoop/test/testcases/KafkaConnectorTestCase.java PRE-CREATION
> test/src/test/java/org/apache/sqoop/integration/connector/kafka/FromRDBMSToKafkaTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/28802/diff/
>
>
> Testing
> -------
>
> Unit tests only for now.
>
>
> Thanks,
>
> Gwen Shapira
>
>
Re: Review Request 28802: SQOOP-1852: Sqoop2: Kafka connector
supporting TO direction
Posted by Gwen Shapira <gs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28802/
-----------------------------------------------------------
(Updated Dec. 9, 2014, 11:31 p.m.)
Review request for Sqoop.
Changes
-------
Added integration test and addressed Abe's concerns
Repository: sqoop-sqoop2
Description
-------
SQOOP-1852: Sqoop2: Kafka connector supporting TO direction
Diffs (updated)
-----
common-test/pom.xml 9fd671c
common-test/src/main/java/org/apache/sqoop/common/test/kafka/KafkaConsumer.java PRE-CREATION
common-test/src/main/java/org/apache/sqoop/common/test/kafka/KafkaLocal.java PRE-CREATION
common-test/src/main/java/org/apache/sqoop/common/test/kafka/TestUtil.java PRE-CREATION
common-test/src/main/java/org/apache/sqoop/common/test/kafka/ZooKeeperLocal.java PRE-CREATION
connector/connector-kafka/pom.xml PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConnector.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConnectorErrors.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConstants.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaLoader.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaToDestroyer.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaToInitializer.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/LinkConfig.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/LinkConfiguration.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/ToJobConfig.java PRE-CREATION
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/configuration/ToJobConfiguration.java PRE-CREATION
connector/connector-kafka/src/main/resources/kafka-connector-config.properties PRE-CREATION
connector/connector-kafka/src/main/resources/sqoopconnector.properties PRE-CREATION
connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/TestConfigValidator.java PRE-CREATION
connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/TestKafkaLoader.java PRE-CREATION
connector/pom.xml da4ed3e
pom.xml e182176
server/pom.xml 1adcca0
server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java dc237b8
test/pom.xml eedb545
test/src/main/java/org/apache/sqoop/test/testcases/KafkaConnectorTestCase.java PRE-CREATION
test/src/test/java/org/apache/sqoop/integration/connector/kafka/FromRDBMSToKafkaTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/28802/diff/
Testing
-------
Unit tests only for now.
Thanks,
Gwen Shapira