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

Allow replacing the scheduled execution time of a unique job #464

Merged
merged 3 commits into from
Apr 19, 2021

Conversation

monteithpj
Copy link
Contributor

This commit adds the capability to update the scheduled execution time of a unique job, similar to :replace_args

Not sure whether this is the nicest way of going about it (if there are other fields that could be replaced in future, this API gets a bit messy) but just went with something very simple to spark discussion.

Any feedback gratefully received 😅

@sorentwo sorentwo self-requested a review April 15, 2021 20:13
@sorentwo sorentwo added area:oss Related to Oban OSS kind:enhancement New feature or request labels Apr 15, 2021
@sorentwo
Copy link
Member

Thanks for opening the PR. This is an interesting use case and it prompts the question: what other things should be replaceable? What about meta, or tags, or priority, etc? I think the API needs to be flexible to support anything that is normally set in Worker.new, otherwise we'll slowly create toward a slough of replace_ keys. WDYT?

@monteithpj
Copy link
Contributor Author

Yeah, that's what I was thinking. I can update to create something like:
Worker.new(args, replace: [:args, scheduled_at])

Would all the options other than :unique, :replace_args and schedule_in would make sense to be replaceable?

@sorentwo
Copy link
Member

That replace syntax looks good!

Would all the options other than :unique, :replace_args and schedule_in would make sense to be replaceable?

Yes, exactly. We can also soft deprecate the replace_args variant and translate that into replace: [:args] internally.

Copy link
Member

@sorentwo sorentwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, nicely done! One small suggestion that will simplify the replace_args support, otherwise this is good to go.

lib/oban/job.ex Show resolved Hide resolved
lib/oban/job.ex Outdated Show resolved Hide resolved
@sorentwo sorentwo merged commit 721bf27 into oban-bg:master Apr 19, 2021
@sorentwo
Copy link
Member

This is great! Thanks for working with me on it 💛

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

Successfully merging this pull request may close these issues.

2 participants