DBZ-8288 AuroraReselectColumnsPostProcessor Implementation#5911
DBZ-8288 AuroraReselectColumnsPostProcessor Implementation#5911gaurav7261 wants to merge 5 commits intodebezium:mainfrom
Conversation
Naros
left a comment
Naros
left a comment
There was a problem hiding this comment.
A few inline suggestions/comments.
...ain/java/io/debezium/connector/postgresql/processors/AuroraReselectColumnsPostProcessor.java
Outdated
Show resolved
Hide resolved
...ain/java/io/debezium/connector/postgresql/processors/AuroraReselectColumnsPostProcessor.java
Show resolved
Hide resolved
debezium-core/src/main/java/io/debezium/processors/reselect/ReselectColumnsPostProcessor.java
Show resolved
Hide resolved
|
@Naros please have a look at pr changes, meanwhile i'm adding test cases as well as we discussed on zulip |
gaurav7261
changed the title
Naros
left a comment
Naros
left a comment
There was a problem hiding this comment.
Just a few nit-picks, I'd prefer that the resolve method return the value and the only assignment opportunity is in the base class.
...ain/java/io/debezium/connector/postgresql/processors/AuroraReselectColumnsPostProcessor.java
Outdated
Show resolved
Hide resolved
...ain/java/io/debezium/connector/postgresql/processors/AuroraReselectColumnsPostProcessor.java
Outdated
Show resolved
Hide resolved
...ain/java/io/debezium/connector/postgresql/processors/AuroraReselectColumnsPostProcessor.java
Outdated
Show resolved
Hide resolved
...ain/java/io/debezium/connector/postgresql/processors/AuroraReselectColumnsPostProcessor.java
Outdated
Show resolved
Hide resolved
debezium-core/src/main/java/io/debezium/processors/reselect/ReselectColumnsPostProcessor.java
Outdated
Show resolved
Hide resolved
debezium-core/src/main/java/io/debezium/processors/reselect/ReselectColumnsPostProcessor.java
Outdated
Show resolved
Hide resolved
debezium-core/src/main/java/io/debezium/processors/reselect/ReselectColumnsPostProcessor.java
Outdated
Show resolved
Hide resolved
|
I reworked the docs just slightly to add an Aurora section. @gaurav7261, do you plan to add the MySQL version here too, or will you be doing that in a separate Jira & PR? |
|
yes @Naros , separate jira and PR for mysql |
jpechane
left a comment
jpechane
left a comment
There was a problem hiding this comment.
@gaurav7261 Thanks a lot.
@Naros @gaurav7261 I left one comment related to the configuration of reselector.
Yet I've one question for both of you. Do we need it to tight it to Aurora? I mean this could be used on arbitrary cloud provider if necessary. I'd propose remove references to aurora in class name, connection name, test name etc. And use a generic name like RedirectedReselectComunsProcessor.
And only in documentation the Aurora uses case will be highligted as the original motivation. WDYT?
| JdbcConnection jdbcConnection = super.resolveJdbcConnection(beanRegistry); | ||
| // create reader connection | ||
| LOGGER.info("Creating reader connection for reselect using reader host: {} and port: {}", readerHost, readerPort); | ||
| JdbcConfiguration newJdbcConfiguration = JdbcConfiguration.copy(jdbcConnection.config()) |
There was a problem hiding this comment.
Do we need even new config options? Maybe it would be better just to take connector level JDBC configuration and override it from options provided by the postprocessor. In that case you'd be able if necessary to override or provide other JDBC connection and conneciton string parameters.
There was a problem hiding this comment.
I like it, and that would reduce the need to create a MySQL variant too 👍
|
@gaurav7261 If we'd go with generic |
|
❌ Developer Certificate of Origin (DCO) check failed. Hi @gaurav7261, please sign off all commits with: If pull request commits are not signed off, the pull request cannot be merged. For more information about why this is required, please see our blog about contribution requirement changes. |
3 participants
https://issues.redhat.com/browse/DBZ-8288