add .prefix options to batch()

[cshum]

Thanks for the great module!

I am looking into adding level-sublevel convention of batch .prefix options.

Since Redis MULTI works across key space, supporting batch across Redisdown instances are basically changing the location mapping.

var dbA = levelup('!a!', { db: redisdown })
var dbB = levelup('!b!', { db: redisdown })

dbA.batch([
  {key: 'foo', value: 'a', type: 'put'},
  {key: 'foo', value: 'b', type: 'put', prefix: dbB } // options.prefix
], function (err) {
  dbA.get('foo', function (err, val) {
    console.log(val) // a
  });
  dbB.get('foo', function (err, val) {
    console.log(val) // b
  });
})

This is definite not a standard leveldown feature, but its more idiomatic to the Redis data structure. What do you think?

[hmalphettes]

@cshum thanks for the good karma, the PR and its tests. Much appreciated!
So far to use redis in cluster mode it was sufficient to place the hash in the database location: #16 (comment)

Does that cover your use case well enough?

Note to self: must make a note about this in the readme.

[cshum]

I have never tried Redis transactions and cluster. Doubt if it is reliable, even if supported?

Should batch prefix simply returns an error if Redis cluster is used?

[hmalphettes]

Oh! I am embarrassed now: I convinced myself that this was related. And that was after morning coffee.

[hmalphettes]

@cshum I understand what the patch is doing and I am not against it.

Are you sure that we must support the 3 different types of prefix?
If we consider this is specific to redis then the string prefix is plenty.
If we allow a redisdown as a prefix we assume it is reusing the same redis client.
And the same remark goes when we use a levelup instance as the value of the prefix.

Do you think we need to support all 3 types of prefixes?
Thanks!

[cshum]

I added 3 types just to cover more cases. The levelup prefix is what I would love to add.
The idea is to switch between sublevels / multiple locations without changing code.

p.s. I wrapped another module with the same options.prefix convention.

Thanks for the input!

[hmalphettes]

I hate to push back on this but I am afraid of covering all those cases: it gets messy to have code that can adapt to a lot of situations; it smells like abstractions are leaking.
In particular casting an object to a levelup instance inside the implementation of a leveldown is problematic to me. I understand the end-game from a level-up API though.

Would it be OK, to support extending redisdown for this?

var redisdown = require('redisdown')(location);
redisdown. __getPrefix = function(prefix) {
....
};

So, I'll add the getPrefix(prefix) function to the prototype of Redisdown and provide the following implementation:

return prefix || this.location;

and one can override it with an implementation where levelup is handled too?

If that is a good way to do it, I'll pull your PR and modify it in this manner.

[cshum]

It's true that handling levelup options inside leveldown smells leaky.
Providing the __getPrefix prototype seems to be the best way of handling this.

Thanks for your consideration on this matter!

[hmalphettes]

@cshum great! Let me know what you think of this: https://github.com/cshum/redisdown/pull/1

[hmalphettes]

Published in redisdown-v0.1.12.
Cheers!