Skip to content

Issue with PoolConnectionProxy connection wrapping and Pool.connection_class #155

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

Closed
nhumrich opened this issue Jun 13, 2017 · 0 comments
Closed
Assignees
Labels

Comments

@nhumrich
Copy link

nhumrich commented Jun 13, 2017

There is an issue with PoolConnectionProxy with how it wraps connection methods, in regards to the addition of the connection_class option you can pass into Pool.

  • asyncpg version: 0.11.0
  • PostgreSQL version: not relevant
  • Python version: 3.6
  • Platform: linux
  • Do you use pgbouncer?: not relevant
  • Did you install asyncpg with pip?: yes
  • Can the issue be reproduced under both asyncio and
    uvloop?
    : Not relevant

in the PoolConnectionProxyMeta class, the new wraps the connection proxy with the methods defined on Connection. Seen on line 30: https://github.com/MagicStack/asyncpg/blob/master/asyncpg/pool.py#L30
The pool then wraps the connection in the proxy class.
What this means is that if connection_class set in the pool overides any public methods such as fetch it doesnt matter, as all methods used will be the definitions of the connection class and not the passed in subclass.
This makes subclassing only useful for overriding underscore methods and not public ones.

In order for subclassing to work, here are some alternatives:

  1. implement __getattr__ to "pass the buck" to the self._con class. I assume the meta class was used instead because of performance reasons. Rather than "looking up" every time. However, the way it is implemented, _dispatch_method_call is called every time, and would in theory be only slightly faster than just using __getattr__ on the class. Actually, here is proof: (code found here)
with metaclass: 1.97497710099924
with __getattr__ 2.10569577699971
  1. We could "wrap" the subclass during __init__ instead of the metaclass. That would Again, be a performance issue, as the wrapping would occur on every new proxy instead of when the class is defined. This is potentially a bigger performance impact.

Any other ideas?

I would be happy to implement this and submit a PR, but want to hear your thoughts on the implementation details.

1st1 added a commit that referenced this issue Jun 30, 2017
Specifically, allow proxied Connection objects to use overridden
public methods.
@1st1 1st1 self-assigned this Jun 30, 2017
@1st1 1st1 added the bug label Jun 30, 2017
@1st1 1st1 closed this as completed in 6ca1f28 Jun 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants