Pool refactoring

[popravich]

This changes are intended to invert pool-to-redis dependency; Redis will be top-level API working
either with pool, or a single connection.

# replace this:
redis_pool = yield from create_pool(...)
with (yield from redis_pool) as redis:
    yield from redis.set('foo', 'bar')

# with this:
redis = yield from create_redis_pool(...)
yield from redis.set('foo', 'bar')

Sub tasks:

• Implement new RedisPool;
• Adopt Redis mixins to work with pool;
• Add address to Connection;
• Impement pool pub/sub;
• Fix pipeline/multiexec execution;
• Fix tests;
• rewrite docs

[coveralls]

Coverage decreased (-1.0%) to 96.2% when pulling 742b7f9 on pool_refactoring into 8726c2e on master.

[coveralls]

Coverage decreased (-0.6%) to 96.615% when pulling fda28de on pool_refactoring into 8726c2e on master.

[codecov-io]

Current coverage is 96.48% (diff: 85.53%)

Merging #129 into master will decrease coverage by 0.49%

@@             master       #129   diff @@
==========================================
  Files            51         50     -1   
  Lines          6160       6267   +107   
  Methods           0          0          
  Messages          0          0          
  Branches        465        481    +16   
==========================================
+ Hits           5974       6047    +73   
- Misses          141        166    +25   
- Partials         45         54     +9   

Powered by Codecov. Last update b4f54fc...d0d25db

[argaen]

Do you think it is possible (or makes sense) to make the initialization of the Client/Pool synchronous like in aiomcache?

https://github.com/aio-libs/aiomcache#getting-started

This way we could avoid patterns like this one where all the functions need to connect to the pool before doing its operation.

[asvetlov]

I think aiomcache has a error in design.
It allows to create a aiomcache.Client with global lifetime extremely easy.
But lifetime of Client should be shorter than loop.
Using coroutines for making connection instances prevents from silly bugs.

Let's consider the following code:

class A:
     client = aiomcache.Client()

What's wrong with it? Hint: think about testing for example.

[argaen]

From a user perspective I prefer this: https://github.com/argaen/aiocache/blob/master/aiocache/backends/memcached.py#L11 than having to have something like https://github.com/argaen/aiocache/blob/master/aiocache/backends/redis.py#L120 and then call it for every method that needs to interact with redis...

Also from testing for me its easier to do:

mymemcached = MemcachedCache()
mymemcached.client = MagicMock()

Than having to patch the _connect function so it returns what the double call to await is supposed to return

[asvetlov]

@argaen what you really need is .post_init() coroutine.

In my snippet I mean not mocking but working with read memcache server in functional tests.

class A:
     client = aiomcache.Client()


async def test_a():
    a = A()
    await a.client.get('key')

test_a will hang forever because test suite creates new event loop for every test.
Proper test code should look like:

async def test_a(loop):
    a = A()
    a.client = aiomcache.Client(loop=loop)
    await a.client.get('key')

but overriding all instances which has grabbed event loop too early is very tedious and error prone.

[popravich]

Do you think it is possible…

Yes, It is possible — you can instatiate RedisPool directly but it is kind of private API;

or makes sense…

No, as this would lead to late errors, i.e. if redis server is unreachable connection errors would
raised only when decorated function get called.