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

Support keyword arguments in iteration jobs #536

Closed
wants to merge 2 commits into from

Conversation

st0012
Copy link
Member

@st0012 st0012 commented Jan 22, 2025

It's now more common to have jobs that take keyword arguments, like MyJob.perform_later(foo: "bar"). But under the current implementation, build_enumerator can only take them as a hash argument instead of keyword arguments:

def build_enumerator(params, cursor:)
  foo = params[:foo]
  # ...
end

This commit improves that by allowing keyword arguments to be passed to build_enumerator and each_iteration as:

def build_enumerator(foo:, cursor:)
  # ...
end

@st0012 st0012 added the enhancement New feature or request label Jan 22, 2025
It's now more common to have jobs that take keyword arguments, like
`MyJob.perform_later(foo: "bar")`. But under the current implementation,
`build_enumerator` can only take them as a hash argument instead of
keyword arguments:

```ruby
def build_enumerator(params)
  foo = params[:foo]
  # ...
end
```

This commit fixes that by allowing keyword arguments to be passed to
`build_enumerator` and `each_iteration`.
@st0012 st0012 force-pushed the make-jobs-take-keyword-arguments branch from 54febaf to 04ab475 Compare January 22, 2025 17:45
@st0012 st0012 closed this Jan 22, 2025
@Mangara
Copy link
Contributor

Mangara commented Jan 29, 2025

@st0012 Can you explain why you closed this? Does it not work? Or was it just unnecessary? Just looking to get some context if someone asks about this in the future.

@st0012
Copy link
Member Author

st0012 commented Feb 11, 2025

I closed this because I later realised that the Shopify apps we care about actually can't adopt the keyword arguments pattern directly. And while this support won't negatively impact those apps either, having it would require job-iteration dropping Ruby 2.6 & 2.7 support at the same time.
So instead of dropping Ruby support for a feature nobody is asking for, I decided to close it and let others to pick it up if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants