-
Notifications
You must be signed in to change notification settings - Fork 10
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
Support Ruby 3.0 kwarg-handling within .delay
API
#7
Changes from all commits
b5bb9b7
f0ab855
77c5061
459cea1
0e0f9c4
fddec5e
7c7769c
ce9b1cc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,8 @@ | ||
module Delayed | ||
class PerformableMethod | ||
attr_accessor :object, :method_name, :args | ||
attr_accessor :object, :method_name, :args, :kwargs | ||
|
||
def initialize(object, method_name, args) | ||
def initialize(object, method_name, args, kwargs) | ||
raise NoMethodError, "undefined method `#{method_name}' for #{object.inspect}" unless object.respond_to?(method_name, true) | ||
|
||
if !her_model?(object) && object.respond_to?(:persisted?) && !object.persisted? | ||
|
@@ -11,6 +11,7 @@ def initialize(object, method_name, args) | |
|
||
self.object = object | ||
self.args = args | ||
self.kwargs = kwargs | ||
self.method_name = method_name.to_sym | ||
end | ||
|
||
|
@@ -23,7 +24,13 @@ def display_name | |
end | ||
|
||
def perform | ||
object.send(method_name, *args) if object | ||
return unless object | ||
|
||
if kwargs.nil? || (RUBY_VERSION < '2.7' && kwargs.empty?) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a proposed PR in the one of the ancestor repos, but for this PR I took a very different strategy, because we want to both maintain Ruby 2.6 support and support the Ruby 3 case where both positional and keyword arguments are present. Holding onto There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But I think that this should resolve collectiveidea/delayed_job#1134 and might be worth upstreaming, if someone gets a chance. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can check if the callee uses kwargs. First get the method object with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this case, I don't think we want it to actually care about the arity of the method it's about to call -- only the arity that was provided to it by the enqueuing call. (For example, if I do So the fix here is really just to prevent |
||
object.send(method_name, *args) | ||
else | ||
object.send(method_name, *args, **kwargs) | ||
end | ||
end | ||
|
||
def method(sym) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing these -- they were already deprecated prior to our fork.