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

[RFC] Allow for Immediate Rejection #53

Closed
erikreedstrom opened this issue Feb 20, 2018 · 3 comments
Closed

[RFC] Allow for Immediate Rejection #53

erikreedstrom opened this issue Feb 20, 2018 · 3 comments

Comments

@erikreedstrom
Copy link

We have use cases where the result of a job may be a failure, but there is no need to retry based on the response. In these cases, we'd like to immediately reject. This will allow us to lower our throughput to rate limited services.

Currently, the code rejects only based on a condition of failure count equaling or exceeding max retries.

See: https://github.com/shinyscorpion/task_bunny/blob/master/lib/task_bunn/worker.ex#L246

Although this works for normal unexpected returns, we suggest adding an additional handler for rejecting explicitly. Specifically, we could handle with two additional function heads:

defp handle_failed_job(state, body, meta, {:error, :reject}), do: handle_failed_job(state, body, meta, {:error, {:reject, nil}})
defp handle_failed_job(state, body, meta,  {:error, {:reject, job_error}}) do
  ...
end

The handler has an additional match on :reject or {:reject, job_error}, directing the process to immediately reject the message where either is returned by the job invocation.

@ono
Copy link
Contributor

ono commented Feb 20, 2018

That sounds very reasonable. I can easily imagine that there would be the case you know retry wouldn't work for the certain situation.

I would suggest to simply use :reject or {:reject, something} instead of wrapping with :error. Most of case, you don't put main business logic to the job module and the return value is used to control TaskBunny behaviour.

What do you think @IanLuites @ricardoperez @Losco @elliotthilaire?

@erikreedstrom
Copy link
Author

@ono Yeah, that was what we initially thought, but wanted to make a suggestion with the least impact on existing code. Having the match in https://github.com/shinyscorpion/task_bunny/blob/master/lib/task_bunny/job_runner.ex#L59-L63 definitely seems cleaner.

@kenfodder
Copy link
Contributor

@erikreedstrom The ability to handle :reject has now been merged and available on 0.3.2

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

No branches or pull requests

3 participants