-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[wip][feat][io] Debezium DB2 source connector for Pulsar #19821
base: master
Are you sure you want to change the base?
Conversation
@devinbost Please add the following content to your PR description and select a checkbox:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just one comment
@dlg99 PTAL
pulsar-io/debezium/db2/src/main/resources/debezium-db2-source-config.yaml
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job!
LGTM with couple of nits + needs proper PR description and title
...ion/src/test/java/org/apache/pulsar/tests/integration/containers/DebeziumDB2DbContainer.java
Outdated
Show resolved
Hide resolved
.../java/org/apache/pulsar/tests/integration/io/sources/debezium/DebeziumDB2DbSourceTester.java
Outdated
Show resolved
Hide resolved
Please follow up with docs update https://github.com/apache/pulsar-site/blob/main/docs/io-debezium-source.md |
Hi @devinbost, I see you've contributed docs along with code changes, thank you! A gentle reminder: have you submitted a PIP for this change? Can you attach the PIP vote link in the PR description? |
pulsar-io/debezium/db2/src/main/java/org/apache/pulsar/io/debezium/db2/package-info.java
Outdated
Show resolved
Hide resolved
...ion/src/test/java/org/apache/pulsar/tests/integration/containers/DebeziumDB2DbContainer.java
Outdated
Show resolved
Hide resolved
…zium/db2/package-info.java Changed oracle to db2 Co-authored-by: Lari Hotari <[email protected]>
Hi @Anonymitaet , I don't think a PIP is typically required for new connectors. |
I don't think that a PIP is required for a new Connector. PIP are important for new APIs and for API changes/breaking changes. Add a new Connector has very little impact, especially for this kind of connectors (based on Debezium) |
...ion/src/test/java/org/apache/pulsar/tests/integration/containers/DebeziumDB2DbContainer.java
Outdated
Show resolved
Hide resolved
I am generally +1 on this work, but I left a comment about the tests |
The pr had no activity for 30 days, mark with Stale label. |
cc @devinbost @eolivelli do we still make progress on this PR? |
It's very close to being finished but I just haven't had time to finish it.
There's just a test that needs to pass. IIRC there was a mismatch in the
number of records the connector was emitting and the number expected due to
something being emitted twice.
…--
Devin Bost
Sent from mobile
On Mon, Jun 12, 2023, 7:57 PM tison ***@***.***> wrote:
cc @devinbost <https://github.com/devinbost> @eolivelli
<https://github.com/eolivelli> do we still make progress on this PR?
—
Reply to this email directly, view it on GitHub
<#19821 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABYTBL3MUEGT24YKV3RYSODXK63H5ANCNFSM6AAAAAAV3VRUT4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
The pr had no activity for 30 days, mark with Stale label. |
@devinbost @eolivelli It looks like this is close, but now there are CI and TestContainer updates needed. |
The pr had no activity for 30 days, mark with Stale label. |
Adds new Debezium DB2 source connector to pulsar-io and includes integration tests.
Fixes #7837
Motivation:
Getting data from IBM DB2 into Pulsar
doc
doc-required
doc-not-needed
doc-complete
Docs at apache/pulsar-site#494
Does this pull request potentially affect one of the following parts:
Dependencies (add or upgrade a dependency)
The public API
The schema
The default values of configurations
The threading model
The binary protocol
The REST endpoints
The admin CLI options
The metrics
Anything that affects deployment
Build process
The biggest change was that we needed to include building a Docker image in the build process for testing the DB2 connector since Debezium hasn't published an actual image with the necessary files. I verified that the DB2 license permits use of these drivers for testing purposes. More details in comments below (#19821 (comment) and #19821 (comment)).
The new Dockerfile was added to
tests/docker-images/debezium-db2-test-image
and was scoped as part of theintegrationTests
profile.Tested against DB2 v11.5
Matching PR in forked repository
devinbost#15