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

Add hooks to CustomizeConnection to allow users to perform actions on connection checkout/in. #131

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

weiznich
Copy link

@weiznich weiznich commented Aug 1, 2022

As discussed in #130

cc @Ten0

///
/// If this method returns an error, the connection will be discarded
#[allow(unused_variables)]
fn on_checkout(&self, conn: &mut C, is_valid_result: Result<(), E>) -> Result<(), E> {
Copy link
Owner

Choose a reason for hiding this comment

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

What are the use cases for wanting to interact with is_valid_result here and has_broken below?

Copy link
Author

Choose a reason for hiding this comment

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

I cannot comment on general use cases here, as that is highly dependent on the ConnectionManager implementation and the underlying connections. Therefore I will only comment on diesel related use cases here.

For on_checkin I see two use cases:

  • Allow to checkin transactions with open transactions into the pool, which is sometimes useful for tests. (Diesel marks connections with open transactions as broken)
  • Trying to close open transactions and "unbreak" connections here

For on_checkout the main use case would be setting up things before checking out the connection, for example opening a dangling transaction. I've included the result of is_valid here for consistency with on_checkin, but I cannot present a diesel related use case to actually use this result. I would guess that there could be a theoretical connection implementation that allows to reconnect the internal connection if some check has failed.

Copy link
Author

Choose a reason for hiding this comment

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

@sfackler Anything that's missing here?

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.

3 participants