Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update debug_lua check to match passed in arg type #765

Merged
merged 3 commits into from
Mar 13, 2023

Conversation

JeremiahChurch
Copy link
Contributor

@JeremiahChurch JeremiahChurch commented Mar 13, 2023

I believe this was introduced with SidekiqUniqueJobs::Script::Caller::normalize_argv

The lua scripts check for a value of "true" for debug_lua but normalize_arg changes that to integer 1 that looks to be typed to a string when it gets to the lua script (I don't know lua in any way so this could very well be wrong)

def normalize_argv(argv)
  argv.each_with_index do |item, index|
    case item
    when FalseClass, NilClass
      argv[index] = 0
    when TrueClass
      argv[index] = 1
    end
  end
end

also remove unused toboolean in lua scripts

I have no idea how to add a test for this.

@mhenrixon mhenrixon enabled auto-merge (squash) March 13, 2023 18:34
@mhenrixon
Copy link
Owner

This looks sane; I don't know if testing is possible since it happens inside redis. One would have to subscribe to the log file of redis.

The change you are referring to was done because of redis-client not supporting additional data types. This was one of the hardest parts of sidekiq 7 compatibility.

@mhenrixon mhenrixon disabled auto-merge March 13, 2023 18:37
@mhenrixon mhenrixon merged commit 763948b into mhenrixon:main Mar 13, 2023
@JeremiahChurch JeremiahChurch deleted the lua_debug branch March 13, 2023 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants