lock method conflict activerecord lock method

[sforce100]

when include Redis::Objects in model。activerecord's lock was override by Redis::Objects。

[byteg]

+1, it's VERY bad, because I don't know how to make DB-level locks now :(
@sforce100 how did you handle this?

[rohanpujaris]

👍 Facing same issue unable to use active record lock method

[nateware]

The idea is that using ActiveRecord's lock method is a very bad idea, and
you should use Redis::Objects instead to do locking. Don't lock your
relational database!

If you want to lock using ActiveRecord, you can instead include only those
classes you want to use in your model. For example, include Redis::Objects::Hash or include Redis::Objects::Counter.

Let me know if you still have issues.

On Mon, Feb 22, 2016 at 3:33 AM, Rohan Pujari notifications@github.com
wrote:

[image: 👍] Facing same issue unable to use active record lock method

—
Reply to this email directly or view it on GitHub
#191 (comment)
.

[byteg]

@nateware why locking DB is a bad idea?(sorry if my question is silly)
My problem is that I do have race conditions problem without using DB-level lock :(
As a workaround I forked redis-objects and renamed the lock method to lock_redis.
Here is my fork:
https://github.com/byteg/redis-objects
@rohanpujaris @sforce100
you might be interested

[rohanpujaris]

@nateware We need row level locking, which is provided by ActiveRecord.
Cool @byteg. But I just mokey patched redis_object gem in our project. Also I think solution given by @nateware will work for us.

[UniIsland]

+1
Innodb supported locking for a reason. And we need it in our app since it is consistent and works on a multiple server setup.
I think redis-object would be better off if namespacing of the class methods injected is supported, see #153.

(sorry for the duplicate issue #196 , i didn't see this one when i submitted it. merge or close it if appropriate, plz)

[110y]

+1
My workaround without monkey patching is below.

# concern
module RedisCacheable
  extend ActiveSupport::Concern

  included do
    class << self
      alias_method(:temp_lock_method, :lock)
    end

    include Redis::Objects

    class << self
      alias_method(:redis_lock, :lock)
      alias_method(:lock, :temp_lock_method)
      remove_method(:temp_lock_method)
    end
  end
end

# model
class User
  include RedisCacheable
end

[vemv]

The idea is that using ActiveRecord's lock method is a very bad idea

With all due ❤️ to the author of such a fantastic gem, I must say that this is a fairly questionable attitude. You're polluting our namespace! Just don't!

This is why things like semver exist. Rename, bump major version, and let us use AR however we want!

Victor

[nateware]

CC @tmsrjs - this issue is a really good one that needs an in-depth consideration. If we want to rename the lock method, we probably need to do a major version bump. I would be ok renaming it to redis_lock or lock_redis or something similar.

[nateware]

I'm closing this as a duplicate of #196 so we can consolidate the conversation there. Thanks.