Add basic resource pool, ported from the one used in riak-ruby-client. - #157
Conversation
|
+1 to merge. A couple minor points are that you might consider adding a test for |
There was a problem hiding this comment.
Is silent failure here a good idea?
There was a problem hiding this comment.
Good catch. I was trying to simplify the logic down below by having a truthy filter if none was specified, but it would be better to raise an error if a non-callable is given.
There was a problem hiding this comment.
I agree. I can't think of a case when that wouldn't be an error.
On Fri, Nov 2, 2012 at 2:45 PM, Sean Cribbs notifications@github.comwrote:
In riak/transports/pool.py:
- @contextmanager
- def take(self, _filter=None, default=None):
"""Claims a resource from the pool for use in a thread-safe,reentrant manner (as part of a with statement). Resources arecreated as needed when all members of the pool are claimed orthe pool is empty.
:param _filter: a filter that can be used to select a memberof the pool:type _filter: callable:param default: a value that will be used instead of callingcreate_resource if a new resource needs to be created"""element = Noneif not callable(_filter):Good catch. I was trying to simplify the logic down below by having a
truthy filter if none was specified, but it would be better to raise an
error if a non-callable is given.—
Reply to this email directly or view it on GitHubhttps://github.com//pull/157/files#r2021554.
|
Other than my quibble, this looks good. |
* Document each test case in the pool suite. * Rename reclaim -> claim_elements in the PoolIterator. * Copy the pool elements inside the lock to avoid some races.
|
The pool primitive is basically ready to go, one last review would be great. |
There was a problem hiding this comment.
It might be nice to include some documentation here specifying that this iteration can block if some element is claimed and not released, and that it does not guarantee any particular ordering over its elements.
It might be safer to add some sort of optional timeout to the next call (not sure that's allowed by the iterator protocol, though), so that you can avoid potential lock situations in multi-threaded programs.
…imer to the docstring.
Add basic resource pool, ported from the one used in riak-ruby-client.
This ports the resource pool from basho/innertube (basically in riak-ruby-client). The intention is that this will eventually replace
ConnectionManager, which is riddled with problems.Features:
contextmanagerto providewithsupport, e.g.with pool.take() as resource.BadResourceexception while a resource is checked out will cause it to be removed from the pool.