-
Notifications
You must be signed in to change notification settings - Fork 3.7k
fix(docs): add missing config in debezium mysql source #14590
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
Conversation
Signed-off-by: Eric Shen <ericshenyuhao@outlook.com>
nodece
left a comment
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
| database.history.pulsar.topic: "history-topic" | ||
| database.history.pulsar.service.url: "pulsar://127.0.0.1:6650" | ||
|
|
||
| pulsar.service.url: "pulsar://127.0.0.1:6650" |
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.
I don't find any code to read pulsar.service.url, could you provide this code? The database.history.pulsar.service.url is correct.
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.
Yes, i just checked the code and also not found the pulsar.service.url. In my test i used the sample YAML on https://pulsar.apache.org/docs/en/io-debezium-source/#configuration-1 but functions gave me an ERROR about the missing config of pulsar.service.url then i noticed the following sample
bin/pulsar-admin source localrun \
--archive connectors/pulsar-io-debezium-mysql-2.9.1.nar \
--name debezium-mysql-source --destination-topic-name debezium-mysql-topic \
--tenant public \
--namespace default \
--source-config '{"database.hostname": "localhost","database.port": "3306","database.user": "debezium","database.password": "dbz","database.server.id": "184054","database.server.name": "dbserver1","database.whitelist": "inventory","database.history": "org.apache.pulsar.io.debezium.PulsarDatabaseHistory","database.history.pulsar.topic": "history-topic","database.history.pulsar.service.url": "pulsar://127.0.0.1:6650","key.converter": "org.apache.kafka.connect.json.JsonConverter","value.converter": "org.apache.kafka.connect.json.JsonConverter","pulsar.service.url": "pulsar://127.0.0.1:6650","offset.storage.topic": "offset-topic"}'
Used the config of "pulsar.service.url": "pulsar://127.0.0.1:6650" and i also added it then passed the test.
I will do the test again to verify it.
nodece
left a comment
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.
I left a comment.
|
Could you provide the error logs? |
|
@gaoran10 @nodece Pulsar and debezium-mysql uses The The status for the sources |
|
It seems that there is a new configuration |
OK, i will test in on |
|
We can add a compatible for old configuration. |
|
The pr had no activity for 30 days, mark with Stale label. |
|
The pr had no activity for 30 days, mark with Stale label. |
|
Closed as no consensus. |
Signed-off-by: Eric Shen ericshenyuhao@outlook.com
Motivation
The YAML sample of debezium mysql source missing a config of
pulsar.service.urlon io-debezium-sourceModifications
Add the missing config of
pulsar.service.urlVerifying this change
(Please pick either of the following options)
This change is a trivial rework / code cleanup without any test coverage.
(or)
This change is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)
Does this pull request potentially affect one of the following parts:
If
yeswas chosen, please highlight the changesDocumentation
Check the box below or label this PR directly (if you have committer privilege).
Need to update docs?
doc-required(If you need help on updating docs, create a doc issue)
no-need-doc(Please explain why)
doc(If this PR contains doc changes)