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.

Sign up for GitHub

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

Jump to bottom

SSL support for Reactive SQL Clients #9469

Merged
merged 6 commits into from
Jun 19, 2020
Merged

SSL support for Reactive SQL Clients #9469

merged 6 commits into from
Jun 19, 2020

Conversation

tsegismont
Copy link
Contributor

@tsegismont tsegismont commented May 19, 2020 •
edited
Loading

@tsegismont
Copy link
Contributor Author

@cescoffier can you please review?

I have been able to test locally with PEM trust store and PEM Key Cert, in both JVM and native modes.

I believe the build platform doesn't provide an SSL protected database for automated testing, does it?

@tsegismont
Copy link
Contributor Author

The build failures look unrelated to the changes, they are probably due to a Quay outage.

@stuartwdouglas
Copy link
Member

The current failure does look related.

@tsegismont tsegismont requested a review from cescoffier May 25, 2020 14:53
@tsegismont
Copy link
Contributor Author

@cescoffier I've pushed the required changes.

@cescoffier @gsmet as I'm looking into adding the same capability to the reactive MySQL client, can I make all the changes in this PR?

@tsegismont tsegismont changed the title SSL support for Reactive Pg Client SSL support for Reactive SQL Clients May 26, 2020
@tsegismont
Copy link
Contributor Author

@cescoffier @gsmet I went ahead and implemented SSL support for both SQL clients.

I have tested locally both Postgres and MySQL.

gsmet
gsmet previously requested changes May 27, 2020
View reviewed changes
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

It looks good overall, I just have a minor gripe for the javadoc. Could you adjust?

@gsmet gsmet added this to the 1.6.0 - master milestone May 27, 2020
@tsegismont tsegismont requested a review from gsmet May 27, 2020 13:58
@tsegismont tsegismont mentioned this pull request May 27, 2020
Merged
@tsegismont
Copy link
Contributor Author

The failure looks unrelated

cescoffier
cescoffier approved these changes Jun 2, 2020
View reviewed changes
Copy link
Member

@cescoffier cescoffier left a comment

Choose a reason for hiding this comment

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

LGTM, I just made a few comments.

@tsegismont
Copy link
Contributor Author

@gsmet @cescoffier anything else needed in this PR?

@tsegismont
Copy link
Contributor Author

@cescoffier @gsmet rebased on master

@tsegismont
Copy link
Contributor Author

@cescoffier failures again that look unrelated

@FroMage FroMage mentioned this pull request Jun 15, 2020
Closed
@FroMage
Copy link
Member

FroMage commented Jun 17, 2020

@gsmet can we dismiss your review? His change was implemented and CI is green.

@FroMage FroMage merged commit 88836ef into quarkusio:master Jun 19, 2020
@tsegismont tsegismont deleted the reactive-pg-ssl branch June 19, 2020 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cescoffier cescoffier cescoffier approved these changes

@gsmet gsmet Awaiting requested review from gsmet

Assignees
No one assigned
Labels
area/documentation area/reactive area/vertx
Projects
None yet
Milestone
1.6.0.CR1
Development

Successfully merging this pull request may close these issues.

vertx-quickstart with ssl Mysql-reactive client is not working on native mode
5 participants
@tsegismont @stuartwdouglas @FroMage @cescoffier @gsmet