This PR adds some missing properties from R2DBC pool:
- max-life-time
- max-acquire-time
- max-create-connection-time
- validation-depth
Comment From: snicoll
Everyone, thank you for all the reviews, I think we're good now.
Comment From: snicoll
@rodolphocouto how is it going? Have you had a chance to look at the review?
Comment From: rodolphocouto
Hey @snicoll, thanks for your review. I started working on that today and I should finish it soon.
Comment From: snicoll
@rodolphocouto thanks for the follow-up. Are you done with d3d38d2? I am asking as I've requested to complete the existing tests to exercise the code you've added. Thanks!
Comment From: rodolphocouto
I just added some tests on R2dbcProperties
. I couldn't find a way to test ConnectionFactory
or ConnectionPool
because they don't expose those properties. Does that make sense to you?
Comment From: snicoll
Reviewing the code, I've noticed 3 properties that have a Duration
of zero to express the absence of a timeout. This looks really odd to me and I am not keen to bring such a default in auto-completion for our users.
We could fix that by having no default value for those 3 properties (the lack of a timeout value meaning no timeout is something we do elsewhere). And only map the value if it's non-null. I've also created an issue and I'd like some feedback before moving forward.
Comment From: snicoll
This is going to be changed in r2dbc-pool
so let's go with the "no default" option for the time being. I'll add a test that will fail as soon as the default changes so that we remember we need to do something about it.