-
Notifications
You must be signed in to change notification settings - Fork 431
Add connection_holder_class to Pool for custom connection handling #1251
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 connection_holder_class to Pool for custom connection handling #1251
Conversation
4a5d365 to
fbd1ef2
Compare
...agicStack#1251 Introduces a `connection_holder_class` parameter to the `Pool` class, enabling the use of custom classes for managing individual connections. This provides more flexibility for handling connection lifecycle, including acquisition and release logic, tailored to specific use cases.
fbd1ef2 to
daa3d96
Compare
If you consider the behavior described in #989 to be a bug in connection management, I also have a modified version of PoolConnectionHolder that addresses it.
This version ensures that connections are not closed below min_size, even in cases where the database drops the connection. I'm happy to submit it as a separate PR if there's interest.
If you consider the behavior described in #989 to be a bug in connection management, I also have a modified version of PoolConnectionHolder that addresses it.
I think #989 is arguably a bug.
Great, I also consider the current behavior to be a bug. In that case, I suggest we fix it as part of this PR — I'll update it a bit later to include the improved PoolConnectionHolder
Great, I also consider the current behavior to be a bug. In that case, I suggest we fix it as part of this PR — I'll update it a bit later to include the improved
PoolConnectionHolder
I would prefer if we separated the fix into a standalone PR.
PoolConnectionHolder is really a fairly low-level implementation detail and I'm a bit wary of turning it into a public API. People already tend to misuse private methods with custom connection/pool classes and things break when we change things (as simple as adding a new argument to a private method). It might be more practical to allow more configurability to Pool via a policy callback or something like that rather than a full subclass.
@elprans, I understand your concerns about making PoolConnectionHolder part of the public API, as it is quite low-level. However, it seems that PoolConnectionHolder is essentially the connection lifecycle management policy within the pool. We might just need to simplify it a bit and make it more suitable for public use, which is what I originally planned to do.
Regarding the idea of adding a separate policy, callback, or something similar, I’ll consider it, but it seems that this would only add extra complexity due to the additional layer of abstraction.
Uh oh!
There was an error while loading. Please reload this page.
This PR introduces the
connection_holder_classparameter to thePoolclass, enabling developers to plug in custom logic for managing individual connections — including acquisition, release, and lifecycle control. This added flexibility is particularly useful for advanced or non-standard use cases where the default connection handling may be insufficient.The change also lays the groundwork for addressing issue #989 by making it possible to implement custom connection management strategies tailored to specific requirements.