Betterment / delayed

a multi-threaded, SQL-driven ActiveJob backend used at Betterment to process millions of background jobs per day
MIT License
156 stars 9 forks source link

Support Ruby 3.0 kwarg-handling within `.delay` API #7

Closed smudge closed 2 years ago

smudge commented 2 years ago

/domain @effron @samandmoore /platform @effron @samandmoore

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 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).

nanda-prbot commented 2 years ago

Needs somebody from @effron and @samandmoore to claim domain review Needs somebody from @effron and @samandmoore to claim platform review

Use the shovel operator to claim, e.g.:

@myname << domain && platform

HOW TO: Claim a Review

effron commented 2 years ago

<<domainLGTM

nanda-prbot commented 2 years ago

Needs somebody from @effron and @samandmoore to claim platform review

Use the shovel operator to claim, e.g.:

@myname << domain && platform

HOW TO: Claim a Review

samandmoore commented 2 years ago

<<platformlgtm

nanda-prbot commented 2 years ago

Approved! :doughnut: :+1: :sparkles:

nanda-prbot commented 2 years ago

Approved! :flags: :gift: :burrito:

nanda-prbot commented 2 years ago

This PR requires additional review because of new changes

Please get another domain review from @effron, or another reviewer with write access if unavailable.

samandmoore commented 2 years ago

nice. glad you tested it :)

domainltm

samandmoore commented 2 years ago

ugh domainlgtm

curse this keyboard!

nanda-prbot commented 2 years ago

This PR requires additional review because of new changes

Please get another domain review from one of @effron, @samandmoore, or another reviewer with write access if unavailable.

smudge commented 2 years ago

Sorry for the review churn! I've been testing this in the context of our largest Rails monorepo, and seem to have ironed out the kinks, so I'd like to cut a new version all in one go.

samandmoore commented 2 years ago

domainlgtm

effron commented 2 years ago

domainLGTM

nanda-prbot commented 2 years ago

Approved! :100: :pizza: :nail_care: