mperham / connection_pool

Generic connection pooling for Ruby
MIT License
1.63k stars 143 forks source link

RFC: Implement `ConnectionPool#then` #138

Closed ianks closed 3 years ago

ianks commented 3 years ago

What?

In Ruby 2.5, Object#then was added as a standard way to "yield self" to the caller. This convention is useful since it allows for library maintainers to abstract access to a value.

For example, Concurrent#Promise implements .then in a way which allow access to a concurrently resolved value:

Concurrent::Promise.execute { :foo }.then { |val| puts val } #=> :foo 

# provides the same value as

:foo.then { |val| puts val } #=> :foo 

The Problem

When using connection-pool in the wild, on multiple occasions I have been unable to directly pass a ConnectionPool wrapped Redis to a library. Most recently, we can't use a ConnectionPool Redis in rack-mini-profiler since it, rightfully, assumes the Redis connection is an actual Redis::Client.

Unfortunately, currently library maintainers like rack-mini-profiler have no easy way to support both raw Redis::Client and a ConnectionPool Redis. In order to be compatible with both, they would have to specifically check if the object responds to .with, and add conditional codepaths. Some library actually do this, which is nice! But overall, it's painful to have to support two codepaths.

Proposed Solution

By adding .then, gems would have a way to support both raw Redis and CP Redis using the same codepath, no conditionals. Offering this API offers an easily adoptable and ergonomic interface for Ruby 2.5+:

# only compatible with Redis::Client, and ConnectionPool::Wrapper
redis.set 'foo' 'bar'

# compatible with Redis::Client, and ConnectionPool Redis 🎉
redis.then { |r| r.set 'foo' 'bar' }

Thoughts?

mperham commented 3 years ago

I will say, this kind of breaks then as we aren't yielding self, we are yielding a connection within the pool. Is this any reason for concern?

mperham commented 3 years ago

BTW good writeup, Ian. Appreciate the explanation, makes it easy to make a quick (hasty?) decision. 😎

ianks commented 3 years ago

I will say, this kind of breaks then as we aren't yielding self, we are yielding a connection within the pool. Is this any reason for concern?

I would not think so. From a developer standpoint, when interacting with connection pool you really want the underlying resource, not the pool typically. And it is more useful.