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

R2DBC: Await closing the driver #4139

Merged
merged 6 commits into from
May 9, 2023
Merged

R2DBC: Await closing the driver #4139

merged 6 commits into from
May 9, 2023

Conversation

hfhbd
Copy link
Collaborator

@hfhbd hfhbd commented May 5, 2023

The current close api of the R2DBC driver does not await the result or failure of the close request. Because we can't use a suspend call, I added a normal plain old callback.

@hfhbd hfhbd self-assigned this May 5, 2023
@hfhbd hfhbd force-pushed the hfhbd/r2dbc-awaitClosing branch from 0d51e47 to ee503b6 Compare May 5, 2023 21:11
@hfhbd hfhbd force-pushed the hfhbd/r2dbc-awaitClosing branch from ee503b6 to 3c548f6 Compare May 8, 2023 09:35
/**
* This callback is called after [close]. It either contains an error or null, representing a successful close.
*/
val closed: (Throwable?) -> Unit,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
val closed: (Throwable?) -> Unit,
val closed: (Throwable?) -> Unit = {},

I feel like in most cases we don't actually need any special handing for this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hm, yes and no. If you don't provide this value, you don't have an option to wait for the close call, because driver.close() will return immediately.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't it return immediately anyway? I would expect that this could still work as a fire-and-forget kind of thing, but that passing in a listener will let you await the completion if you really need it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Of course, this call isn't suspending or blocking, see the tests for a waiting implementation.
So yes, you still need to implement the listener (or we could provide an CoroutineScope overload).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sounds good to me either way

@hfhbd hfhbd requested a review from dellisd May 8, 2023 14:45
@hfhbd hfhbd merged commit 01ce2b4 into master May 9, 2023
6 checks passed
@hfhbd hfhbd deleted the hfhbd/r2dbc-awaitClosing branch May 9, 2023 15:44
hfhbd added a commit that referenced this pull request May 19, 2023
* R2DBC: Await closing the driver

* Fix compiler error due to rebasing

* Add coroutines overload

* Add doc

* Refactor test code with use

* Spotless...

---------

Co-authored-by: hfhbd <[email protected]>
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

2 participants