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

Ruby 3 support #6

Closed
valerauko opened this issue Nov 27, 2021 · 2 comments · Fixed by #7
Closed

Ruby 3 support #6

valerauko opened this issue Nov 27, 2021 · 2 comments · Fixed by #7

Comments

@valerauko
Copy link

Ruby 3 changed the way it handles keyword arguments, and is a roadblock with delayed_job. I checked the relevant bits in the source of this gem (at least the bits that used to be relevant in delayed_job) and didn't see any changes or fixes for Ruby 3 keyword argument handling.

def method_missing(method, *args)
Job.enqueue({ payload_object: @payload_class.new(@target, method.to_sym, args) }.merge(@options))
end

def perform
object.send(method_name, *args) if object
end

While the gemspec suggests Ruby 3 should work, does it really?

@smudge
Copy link
Member

smudge commented Nov 29, 2021

Great callout! Thank you!

Our focus for this gem has been to shift it towards using ActiveJob as the primary end-developer API, and ActiveJob performables already fully support Ruby 3.0 kwargs.

That said, we have not removed or reworked the existing Delayed::MessageSending APIs. Therefore, we should consider these APIs fully supported (at least, as long as we don't officially say they aren't), and so it would only make sense to fix these Ruby 3 compatibility issues. I put together a proposed code change (#7) to address this.

Longer term, we may choose to extract all of the Delayed::MessageSending behaviors into an optional gem, or remove them entirely in favor of something else (to be determined - maybe an ActiveJob-based equivalent of .delay). For now, though, hopefully these Ruby 3.0 fixes will tide us over!

@valerauko
Copy link
Author

valerauko commented Nov 30, 2021

Oh if the ActiveJob (perform_later?) API works with Ruby 3 this issue is moot. .delay is a pretty dangerous construct so if you say it's not supported I don't think that's an issue.

@smudge smudge closed this as completed in #7 Nov 30, 2021
smudge added a commit that referenced this issue Nov 30, 2021
Resolves #6. While we already fully support Ruby 3.0 kwargs via ActiveJob, the legacy `delay` and `handle_asynchronously` APIs did not support the new [separation of positional and keyword arguments](https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/) introduced in 2.7 and enforced in 3.0.

This means that methods accepting both positional and keyword arguments (e.g. `def foo(a, b:)`) would fail with a `wrong number of arguments (given 2, expected 1; required keyword:)` error on Ruby 3. In order to resolve this, this PR changes `Delayed::PerformableMethod` to handle kwargs separately from args, with a backwards compatibility check for any Ruby 2.6 methods that do not accept keyword arguments. It should also support jobs that were enqueued by the prior `delayed` gem version (where the new `kwargs` accessor would be nil, and `args` contains the kwargs as its last item).
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 a pull request may close this issue.

2 participants