-
Notifications
You must be signed in to change notification settings - Fork 2.1k
[oracle]#1244 Oracle URL format is inconsistent with the debezium def… #1258
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
[oracle]#1244 Oracle URL format is inconsistent with the debezium def… #1258
Conversation
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.
Thanks @yanghuaiGit for the contribution, the pr looks generally good,I left some comments.
...-connector-oracle-cdc/src/main/java/com/ververica/cdc/connectors/oracle/OracleValidator.java
Outdated
Show resolved
Hide resolved
...-connector-oracle-cdc/src/main/java/com/ververica/cdc/connectors/oracle/OracleValidator.java
Outdated
Show resolved
Hide resolved
6bb2304
to
c50634d
Compare
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.
Thanks @yanghuaiGit for the update, the change is in good shape, I only left two minor suggestions. I think we can merge this PR once you addressed them.
...oracle-cdc/src/main/java/com/ververica/cdc/connectors/oracle/util/JdbcConfigurationUtil.java
Show resolved
Hide resolved
...le-cdc/src/main/java/com/ververica/cdc/connectors/oracle/table/OracleTableSourceFactory.java
Show resolved
Hide resolved
I have same problem to support SID and SERVICENAME to connect ORACLE. Why don't we add one "type" way ?
|
Usually the URL username password parameter is needed to connect to a data source, so you should expose the URL parameter to the customer rather than using a type parameter to control the generated URL format |
@modoojunko please open another issue or discussion, we should not discuss unrelated issue the under this PR. |
...dc/src/test/java/com/ververica/cdc/connectors/oracle/table/OracleTableSourceFactoryTest.java
Show resolved
Hide resolved
...or-oracle-cdc/src/main/java/com/ververica/cdc/connectors/oracle/table/OracleTableSource.java
Show resolved
Hide resolved
...or-oracle-cdc/src/main/java/com/ververica/cdc/connectors/oracle/table/OracleTableSource.java
Show resolved
Hide resolved
flink-connector-oracle-cdc/src/main/java/com/ververica/cdc/connectors/oracle/OracleSource.java
Outdated
Show resolved
Hide resolved
flink-connector-oracle-cdc/src/main/java/com/ververica/cdc/connectors/oracle/OracleSource.java
Show resolved
Hide resolved
...or-oracle-cdc/src/main/java/com/ververica/cdc/connectors/oracle/table/OracleTableSource.java
Outdated
Show resolved
Hide resolved
...le-cdc/src/main/java/com/ververica/cdc/connectors/oracle/table/OracleTableSourceFactory.java
Outdated
Show resolved
Hide resolved
...oracle-cdc/src/main/java/com/ververica/cdc/connectors/oracle/util/JdbcConfigurationUtil.java
Outdated
Show resolved
Hide resolved
...oracle-cdc/src/main/java/com/ververica/cdc/connectors/oracle/util/JdbcConfigurationUtil.java
Outdated
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.
Thank @yanghuaiGit for the nice work, I only left one doc comment
7de1587
to
d323bf6
Compare
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.
Thanks @yanghuaiGit for the contribution, LGTM
flinkcdc oracle connector just support sid,but debezium default support serviceName,so we should set property 'database.url'