cookpad / arproxy

Arproxy is a proxy between ActiveRecord and database adapter
https://github.com/cookpad/arproxy
MIT License
327 stars 26 forks source link

Fix ArgumentError on activerecord 7.0 #21

Closed r7kamura closed 2 years ago

r7kamura commented 2 years ago

The #execute method may have keyword-arguments, particular at MySQL adapter since activerecord 7.0.0. See also: https://github.com/rails/rails/commit/7fc174aadaefc2c0a8a6b7c8a7599dd9ca04811f

Resolve https://github.com/cookpad/arproxy/issues/20.

r7kamura commented 2 years ago

I understand this would require a change in #execute method signature on all decendants under Arproxy::Base in order to use with ActiveRecord 7+.

That's true at least on the MySQL adapter users. In fact, I need to change my Rails app code like this:

diff --git a/lib/multiple_database_connection_logger.rb b/lib/multiple_database_connection_logger.rb
index cc38fb6f34c..24b7d278aff 100644
--- a/lib/multiple_database_connection_logger.rb
+++ b/lib/multiple_database_connection_logger.rb
@@ -1,5 +1,5 @@
 class MultipleDatabaseConnectionLogger < Arproxy::Base
-  def execute(sql, name = nil)
+  def execute(sql, name = nil, **)
     role = ActiveRecord::Base.current_role
     name = "#{name} [#{role}]"
     super
sorah commented 2 years ago

@mirakui gentle reminder or I'll merge at my discretion soon

r7kamura commented 2 years ago

@mirakui @sorah I'm waiting for the merge of this pull request. Is there anything more I can do?

sorah commented 2 years ago

I assumed @mirakui would merge this 😓 Sorry for blocking you long time

mirakui commented 2 years ago

@r7kamura I'm sorry for blocking you 🙇 I've just released arproxy-0.2.9 gem with this patch.

r7kamura commented 2 years ago

Thank you for responding! ✨💎✨