Improve gem configuration about brpoplpush-redis_script HOT 5 CLOSED

Good points! I'll get back to you with some thoughts when I have a clear and calm mind

from brpoplpush-redis_script.

I started to have a look to the configuration but the basing the configuration on a Concurrent::MutableStruct will restrict the scope of the improvement I was considering.

In particular, the latest version of its abstraction (https://github.com/ruby-concurrency/concurrent-ruby/blob/04e9390c4182049eb23a0cd736199fb17397d77c/lib/concurrent/synchronization/abstract_struct.rb) is not implementing the keyword_init behavior introduced in the ruby standard library (https://github.com/ruby/ruby/blob/58dfea0c8de29cbad35621847c04372b779c5f5d/struct.c#L483-L488).

What I may suggest is to let the user configure differently the SCRIPT_DIRECTORY coming up with something like:

RedisScript.configure do |config|
  config.add_scripts_path 'foo/bar'
  config.add_script :my_name, 'foo/bar/baz.lua'
end

RedisScript.configuration.script_paths # Concurrent::Hash
# {script_name1: file_path1, script_name2: file_path2}

After looking into the Brpoplpush::RedisScript::Template (in particular the partials feature) and the way the Brpoplpush::RedisScript::Client (script_path/script_source methods) this may be postponed we will really need this flexibility.

I assume moreover that:

• we should preserve the benefits coming from Concurrent::MutableStruct
• this change may introduce conflicts in the for files with the same name in different folders

from brpoplpush-redis_script.

Regarding the configuration I still have a doubt.
Assume that sidekiq_unique_jobs will you this gem setting a default SCRIPT_DIRECTORY, what would happen if I use this gem standalone for loading a custom folder in my project?
Would it conflixt with the configuration in my other dependency? If this is case, we should definitely add support for multiple paths since day 0 😄

from brpoplpush-redis_script.

I’ll show you exactly how to do this. Give me half an hour or so.

from brpoplpush-redis_script.

@mberlanda have a look at #12 please.

from brpoplpush-redis_script.