gregoriusxu / booksleeve

Automatically exported from code.google.com/p/booksleeve
Other
0 stars 0 forks source link

Changes to the API #25

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
I realize this may be a problem to implement (breaking changes etc.), but I 
really hate the fact that all the Redis methods contain the following 2 
parameters:
int db, and bool queueJump
It would be nice if those parameters worked the way transactions are working 
now, these would be methods of the Connection object which would return an 
object which supports the same API. e.g.:

Connection.SelectDb(0).Hashes.Get(/*no need for int DB here*/,...)
or
Connection.SelectDb(0).QueueJump.Get(..);

This will allow to remove those 2 parameter across the API (making it much 
cleaner) and will also allow to cache the object returned from SelectDb for 
reuse (most apps don't change DB during execution)

Original issue reported on code.google.com by eyalpost on 29 Oct 2012 at 8:35

GoogleCodeExporter commented 8 years ago
The queueJump parameter is optional and can be ignored, unless you're on a 
down-version compiler.

The presence of the db parameter *at some level* is essential when you consider 
that it is intended to be used as a free-threaded multiplexer, i.e. accessed 
from multiple threads concurrently. I can see what you mean, though, with a 
method return a wrapper object (with the db pre-determined), that forwards the 
requests on. I guess it comes down to the overhead of supplying an extra 
parameter vs the overhead of calling something like "SelectDB" (or storing the 
wrapped instance) each time.

In which scenarios are these being problematic? is it mainly at the "stylistic" 
level?

Original comment by marc.gravell on 29 Oct 2012 at 9:23

GoogleCodeExporter commented 8 years ago
Hi Marc,

thanks for answering and thanks for a great library! :)
Whenever I see an API that has the same parameter in ALL methods it usually 
means it can be moved to an different level.
I agree queueJump isn't so much of a problem because it's optional (although 
its a bit annoying if you'll need to add another overload with a boolean 
parameter)
regarding SelectDb, it just makes the calling code much less cleaner. We need 
to pass an additional parameter (which in our case comes from a config) 
whenever we call the API. The db parameter isn't really essential even when 
multiplexing because I can always store the wrapper I received from SelectDb 
instead of storing the RedisConnection object. 

I wouldn't call it a "stylistic" issue. For me, it's like having to pass the 
connection string to a database on every call to the database instead of 
getting a Connection object and storing it for multiple uses.

Original comment by eyalpost on 30 Oct 2012 at 8:41

GoogleCodeExporter commented 8 years ago
All of this will be implemented in a future version:

- db-less: will be needed for redis-server
- queue-jump: is largely deprecated by 1.3

So: both of these will happen.

Original comment by marc.gravell on 25 May 2013 at 5:44

GoogleCodeExporter commented 8 years ago
Marc,

Any update on adding db-less parameters to the API? When I normally use Redis, 
we use the multi-tennant ID as part of the *key*, not the DB selection. 
Additionally, when we run tests, we actually hit Redis. In our base "RedisTest" 
class, we set the DB to something other than the development DB (15) so that 
the test data does not conflict with the developers local data. 

Because we're completely switching away from ServiceStack.Redis (because of the 
new license model), we're evaluating BookSleeve. ServiceStack.Redis had the 
ability to select the DB after the connection was created.

Original comment by adrian.p...@gmail.com on 11 Nov 2013 at 7:30

GoogleCodeExporter commented 8 years ago
Yes! I have started spiking out the key pieces of the vNext API, bringing 
together:

- db-less operations (or rather, db-scoped at a higher level)
- string/byte[] keys
- redis-cluster support

The 1st and 3rd are tangentially related since redis-cluster does not support 
databases. Nothing works yet, but: there is work in progress.

Original comment by marc.gravell on 11 Nov 2013 at 11:54

GoogleCodeExporter commented 8 years ago
This is addressed in StackExchange.Redis, the successor to BookSleeve

Original comment by marc.gravell on 20 Mar 2014 at 12:06