Fix on Ruby 3 #283
Fix on Ruby 3 #283
Conversation
mlarraz
force-pushed
the
ruby_3_support
branch
8 times, most recently
from
4827e64 to
253a6b8
Compare
mlarraz
force-pushed
the
ruby_3_support
branch
21 times, most recently
from
2407938 to
a20386d
Compare
mlarraz
force-pushed
the
ruby_3_support
branch
11 times, most recently
from
ecdb1fc to
c8ec6f5
Compare
mlarraz
force-pushed
the
ruby_3_support
branch
from
c8ec6f5 to
bc54612
Compare
|
|
||
| # Each method the Makara::Proxy needs to hijack should be redefined in the underlying connection. | ||
| # The new definition should allow for the proxy to intercept the invocation if required. | ||
| @proxy.class.hijack_methods.each do |meth| | ||
| extension << %Q{ | ||
| def #{meth}(*args, &block) | ||
| method_call = RUBY_VERSION >= "3.0.0" ? "public_send(#{meth.inspect}, ...)" : "#{meth}(*args, &block)" |
There was a problem hiding this comment.
We need public_send here because foo.bar=(...) is not valid syntax in Ruby 3.0 (even though foo.bar(...) is)
mlarraz
force-pushed
the
ruby_3_support
branch
from
6e68673 to
326d088
Compare
| @@ -73,7 +73,13 @@ | |||
| Test::User.exists? # flush other (schema) things that need to happen | |||
|
|
|||
| con = connection.replica_pool.connections.first | |||
| expect(con).to receive(:exec_query).with(/SELECT\s+1\s*(AS one)?\s+FROM .?users.?\s+LIMIT\s+.?1/, any_args).once.and_call_original | |||
|
|
|||
| expect(con).to receive(:exec_query) do |query| | |||
There was a problem hiding this comment.
Seems like this change is causing a failure for this matrix run: https://github.com/instacart/makara/pull/283/checks?check_run_id=1730065278
There was a problem hiding this comment.
No, this spec flaps on jruby for some reason. It's the same on master now.
There was a problem hiding this comment.
| elsif con.respond_to?(m, true) | ||
| con.__send__(m, *args, &block) | ||
| if con.respond_to?(m, true) | ||
| con.send(m, *args, &block) |
There was a problem hiding this comment.
Subtle regression here. We'll now call a private #foo method on the ConnectionWrapper instead of calling the public #foo method on the wrapped ConnectionAdapter. That's because using #send will dispatch the private method immediately whereas #public_send will fall back to method_missing, which is where the Makara ConnectionWrapper talks to the AR ConnectionWrapper.
Example
c.send(:select, "select 1")
*** TypeError Exception: wrong argument type String (expected Array)
c.public_send(:select, "select 1")
(2.1ms) select 1
#<ActiveRecord::Result:0x00007f8784b4e7f0 @columns=["1"], @rows=[[1]], @hash_rows=nil, @column_types={}>This is the cause of #326
There was a problem hiding this comment.
Thanks for catching this. TIL about #send not hitting #method_missing.
If you can send a PR and maybe add a spec to prevent a regression I'll get it merged ASAP.
There was a problem hiding this comment.
Sure thing. Thanks @mlarraz.
Because of how much method delegation and metaprogramming this gem uses, there were a number of issues due to the Ruby 3 keyword argument change.
Most are resolvable by adding
ruby2_keywords, but the stuff usinginstance_evalis a bit more involved.