Skip to content

Conversation

@dlg99
Copy link
Contributor

@dlg99 dlg99 commented Jul 30, 2021

Motivation

Add Debezium Source for Oracle.

Modifications

Added Debezium Source for Oracle DB.
Added integration test.
Oracle JDBC is not included into nar (licensing issue) so the end user will have to package it.
Dropping jars into the pulsar/lib directory is not enough (connector classpath doesn't get it) so for the integration test I ended up updating the nar file with jar uf.

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

  • Added integration test
$ $ mvn test -f tests/pom.xml -DintegrationTestSuiteFile=pulsar-io-ora-source.xml -DintegrationTests -Dtest=org.apache.pulsar.tests.integration.io.sources.debezium.PulsarDebeziumOracleSourceTest  -DtestRetryCount=0
...
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running org.apache.pulsar.tests.integration.io.sources.debezium.PulsarDebeziumOracleSourceTest
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 445.938 s - in org.apache.pulsar.tests.integration.io.sources.debezium.PulsarDebeziumOracleSourceTest
[INFO]
[INFO] Results:
[INFO]
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for Apache Pulsar :: Tests 2.9.0-SNAPSHOT:
[INFO]
[INFO] Apache Pulsar :: Tests ............................. SUCCESS [  1.948 s]
[INFO] Apache Pulsar :: Tests :: Integration .............. SUCCESS [07:34 min]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  07:36 min

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): no
  • The public API: no
  • The schema: no
  • The default values of configurations: no
  • The wire protocol: no
  • The rest endpoints: no
  • The admin cli options: no
  • Anything that affects deployment: no

Documentation

For contributor

For this PR, do we need to update docs?

Need to add description of the new connector; TBD (or will create an issue later).
Need to document requirement to download and package Oracle JDBC driver.

@Anonymitaet Anonymitaet added the doc-required Your PR changes impact docs and you will update later. label Aug 3, 2021
@Anonymitaet
Copy link
Member

@dlg99 many thanks for providing doc info!
When adding docs, you can take the following guides as references:

@zymap zymap requested review from eolivelli, lhotari and sijie August 4, 2021 02:49
Copy link
Member

@sijie sijie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before reviewing this, is there any license issues we should clear out?

@merlimat
Copy link
Contributor

merlimat commented Aug 5, 2021

This is depending on Oracle JDBC driver https://mvnrepository.com/artifact/com.oracle.ojdbc/ojdbc8/19.3.0.0 that is licensed with "Oracle Free Use Terms and Conditions (FUTC)". It's not clear if this license is compatible with ASLv2.

@dlg99
Copy link
Contributor Author

dlg99 commented Aug 24, 2021

@sijie @merlimat I removed dependency on Oracle JDBC and updated tests/docker-images/latest-version-image/Dockerfile to download ojdbc jars for the integration test. Users will have to deal with this.
According to https://issues.apache.org/jira/browse/LEGAL-526 FUTC is a no-go for Apache projects.

@eolivelli
Copy link
Contributor

What about creating one dedicated CI job only for Oracle CDC ?

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great work
I left some comments about hardcoding 2.9.0-SNAPSHOT, PTAL

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
great work !

@Anonymitaet
Copy link
Member

Thanks for your great contribution.
Want users to know your awesome code changes? Do not forget to add docs accordingly! And you can ping me to review the docs. Let's grow Pulsar together!

Copy link
Contributor

@merlimat merlimat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@eolivelli
Copy link
Contributor

@sijie are you okay with merging this PR now ? it looks like licensing issues have been addressed.

@dlg99
Copy link
Contributor Author

dlg99 commented Sep 7, 2021

ping @sijie

@eolivelli eolivelli requested a review from sijie September 13, 2021 15:49
@eolivelli eolivelli merged commit e2bc52d into apache:master Sep 13, 2021
@lhotari
Copy link
Member

lhotari commented Sep 14, 2021

@eolivelli @dlg99 The new integration test seems to be flaky. Here's one sample failure:
https://github.com/apache/pulsar/pull/12027/checks?check_run_id=3591596786#step:13:27471

Error:  testDebeziumOracleDbSource(org.apache.pulsar.tests.integration.io.sources.debezium.PulsarDebeziumOracleSourceTest)  Time elapsed: 1,800.028 s  <<< FAILURE!
org.testng.internal.thread.ThreadTimeoutException: Method org.apache.pulsar.tests.integration.io.sources.debezium.PulsarDebeziumOracleSourceTest.testDebeziumOracleDbSource() didn't finish within the time-out 1800000

dlg99 added a commit to dlg99/pulsar that referenced this pull request Sep 24, 2021
eolivelli pushed a commit to datastax/pulsar that referenced this pull request Sep 27, 2021
@dlg99 dlg99 deleted the dbz-oracle branch October 14, 2021 23:31
@Anonymitaet
Copy link
Member

Hi @dlg99 for the doc side, I think we already documented "the requirement to download and package Oracle JDBC driver" here(https://pulsar.apache.org/docs/en/next/io-debezium-source/#example-of-oracle)?

@dlg99
Copy link
Contributor Author

dlg99 commented Nov 12, 2021

@Anonymitaet correct, see #12213 for the docs

@Anonymitaet Anonymitaet added doc-complete Your PR changes impact docs and the related docs have been already added. and removed doc-required Your PR changes impact docs and you will update later. labels Nov 12, 2021
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Feb 28, 2022
bharanic-dev pushed a commit to bharanic-dev/pulsar that referenced this pull request Mar 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-complete Your PR changes impact docs and the related docs have been already added.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants