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

Expose validate-on-borrow provided by Agroal through Quarkus #41622

Merged
merged 1 commit into from
Jul 4, 2024
Merged

Expose validate-on-borrow provided by Agroal through Quarkus #41622

merged 1 commit into from
Jul 4, 2024

Conversation

mkrueger92
Copy link
Contributor

This fixes #41616.

@mkrueger92 mkrueger92 force-pushed the feature/Expose_validate-on-borrow branch from 66f79db to a237883 Compare July 2, 2024 13:38
gsmet
gsmet approved these changes Jul 2, 2024
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.

I think it makes a lot of sense to allow this but I will let @yrodiere have the final say :).

@gsmet gsmet requested a review from yrodiere July 2, 2024 13:51
yrodiere
yrodiere reviewed Jul 2, 2024
View reviewed changes
Copy link
Member

@yrodiere yrodiere left a comment

Choose a reason for hiding this comment

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

Suggested some more documentation.

The implementation seems fine, but I don't quite understand the feature or use case (sorry!), so let's discuss it there: #41616 (comment)

...ions/agroal/runtime/src/main/java/io/quarkus/agroal/runtime/DataSourceJdbcRuntimeConfig.java Outdated Show resolved Hide resolved
@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@mkrueger92 mkrueger92 force-pushed the feature/Expose_validate-on-borrow branch 2 times, most recently from 33a430d to cf6e71b Compare July 2, 2024 15:24
yrodiere
yrodiere reviewed Jul 2, 2024
View reviewed changes
Copy link
Member

@yrodiere yrodiere left a comment

Choose a reason for hiding this comment

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

Please squash the commits and once that's done, +1 from me :)

@mkrueger92
Expose validate-on-borrow
bb875c8 
Expose the option to validate the connection on borrow as supported by Agroal.

Fix quarkusio#41616
@mkrueger92 mkrueger92 force-pushed the feature/Expose_validate-on-borrow branch from cf6e71b to bb875c8 Compare July 3, 2024 08:01
@geoand geoand requested a review from yrodiere July 3, 2024 08:02
@gsmet gsmet merged commit ac13706 into quarkusio:main Jul 4, 2024
43 checks passed
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label Jul 4, 2024
@quarkus-bot quarkus-bot bot added this to the 3.13 - main milestone Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gsmet gsmet gsmet approved these changes

@yrodiere yrodiere yrodiere approved these changes

Assignees
No one assigned
Labels
area/agroal kind/enhancement New feature or request
Projects
None yet
Milestone
3.13.0.CR1
Development

Successfully merging this pull request may close these issues.

Expose validate-on-borrow provided by Agroal through Quarkus
3 participants
@mkrueger92 @yrodiere @gsmet