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

Adds feature for users to customize connections created by the pool #89

Merged
merged 8 commits into from
Dec 14, 2020
Merged

Adds feature for users to customize connections created by the pool #89

merged 8 commits into from
Dec 14, 2020

Conversation

agersant
Copy link
Contributor

@agersant agersant commented Dec 12, 2020

Implementation for #88

I ended up running into not too many issues!

bb8/src/api.rs Outdated Show resolved Hide resolved
bb8/src/inner.rs Outdated Show resolved Hide resolved
@djc
Copy link
Owner

djc commented Dec 12, 2020

Overall this is looking good, thanks for working on it!

@agersant
Copy link
Contributor Author

I think this is ready for another look. Removing on_release made it a much smaller PR too.

bb8/src/inner.rs Outdated
Comment on lines 179 to 185
let conn = match shared.manager.connect().await {
Err(e) => Err(e),
Ok(mut c) => match self.on_acquire_connection(&mut c).await {
Err(e) => Err(e),
Ok(_) => Ok(c),
},
};
Copy link
Owner

Choose a reason for hiding this comment

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

The repeated Err(e) => Err(e) arms can probably be factored out by using some combinators, can you look into that? And while you're in there, please add an empty line between first let conn = match and match conn { blocks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. Looks a bit more compact.

@djc
Copy link
Owner

djc commented Dec 14, 2020

This is looking good, some minor style things and then I think this is good to merge!

@djc djc merged commit fc39fd3 into djc:main Dec 14, 2020
@djc
Copy link
Owner

djc commented Dec 14, 2020

Thanks!

@djc djc mentioned this pull request Dec 14, 2020
djc added a commit that referenced this pull request Dec 25, 2020
djc added a commit that referenced this pull request Dec 25, 2020
djc added a commit that referenced this pull request Dec 26, 2020
djc added a commit that referenced this pull request Dec 26, 2020
djc added a commit that referenced this pull request Dec 27, 2020
@jakajancar
Copy link

Is this supposed to run when the connection is first created, or every time it's checked out?

  • From the code, it seems to me when the connection is first created.
  • Based on the docs ("customizer to customize newly checked out connections") and the fact this was suggested as a workaround for Postgres: DISCARD ALL #73, it seems to imply the latter, on every check out.

Also would be nice to document that it applies to dedicated_connection(), which I'm not sure if it's considered "checked out" or not.

@djc
Copy link
Owner

djc commented Jan 25, 2023

#73 is still open for a reason. This was indeed a solution to a different problem, referencing #88.

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