Skip to content
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

Debezium SQL Server Connector native support #1193 #1278

Merged
merged 1 commit into from Jun 3, 2020

Conversation

JiriOndrusek
Copy link
Contributor

Issue:#1193

[ ] An issue should be filed for the change unless this is a trivial change (fixing a typo or similar). One issue should ideally be fixed by not more than one commit and the other way round, each commit should fix just one issue, without pulling in other changes.
[ ] Each commit in the pull request should have a meaningful and properly spelled subject line and body. Copying the title of the associated issue is typically enough. Please include the issue number in the commit message prefixed by #.
[ ] The pull request description should explain what the pull request does, how, and why. If the info is available in the associated issue or some other external document, a link is enough.
[ ] Phrases like Fix # or Fixes # will auto-close the named issue upon merging the pull request. Using them is typically a good idea.
[ ] Please run mvn process-resources -Pformat (and amend the changes if necessary) before sending the pull request.
[ ] Contributor guide is your good friend: https://camel.apache.org/camel-quarkus/latest/contributor-guide.html

@JiriOndrusek JiriOndrusek force-pushed the 193_debezium_sqlserver branch 4 times, most recently from 07c0dbb to f9608c5 Compare June 1, 2020 10:43
@JiriOndrusek JiriOndrusek marked this pull request as ready for review June 1, 2020 12:02
Copy link
Contributor

@jamesnetherton jamesnetherton left a comment

Choose a reason for hiding this comment

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

Looks mostly ok to me.

Some minor things to fix up & improve.

SQL server may require encryption. In that case `quarkus.ssl.native=false` parameter has to be added during execution.
See https://quarkus.io/guides/native-and-ssl for more details.

=== Integration testing
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe move this section into a dedicated README in the debezium itest module. It's not really required information for the most users.

@@ -39,8 +40,11 @@
<!-- mvn process-resources -Pformat from the root directory -->
<mvnd.builder.rule>camel-quarkus-debezium-postgres-deployment,camel-quarkus-debezium-mysql-deployment,camel-quarkus-support-policy-deployment</mvnd.builder.rule>
Copy link
Contributor

Choose a reason for hiding this comment

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

camel-quarkus-debezium-sqlserver-deployment seems to be missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jamesnetherton Both changes are added to the PR.

@jamesnetherton
Copy link
Contributor

@JiriOndrusek please can you rebase and resolve conflicts.

@JiriOndrusek
Copy link
Contributor Author

@jamesnetherton rebased and conflicts resolved

@jamesnetherton jamesnetherton merged commit 7c1de37 into apache:master Jun 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants