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

Let users delete a specific job from a queue in the web UI #66

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

reprah
Copy link

@reprah reprah commented Aug 17, 2014

This feature places a "Delete" button next to each job in a queue. When that button is clicked, the enqueued job is removed and the user is taken back to the queue's page.

Summary of my changes:

  • Added a JobsController with one action (destroy) that deletes a specific job. My reasoning behind creating a new controller is I think it's more RESTful than making a "delete_job" action in the QueuesController
  • Added functional tests for my JobsController
  • Added a new column called "Actions" on the queue's show page where the delete button for each job is housed

Here's an issue I want to bring up... since deleting a specific job relies on Resque::Job's class method destroy (see here, please) some things might behave unexpectedly. Here's what I mean:

  • As far as I know, Resque determines which job to delete based on a combination of the queue it's in, its job class, and the arguments to be passed to the job. We don't have an ID number. We might have multiple jobs with those same characteristics, and trying to delete one would delete ALL of them
  • If no job arguments are passed to that method, Resque deletes all the jobs in that queue with that job class

There's one gem I found that gives UUIDs to Resque jobs (resque-status), but this gem would have to be present at the time someone creates a job and everyone would have to use this gem. We can't rely on that obviously.

The ball's in your court here, just let me know if there's a way I can improve this PR or if the above caveats make this a sort of undesirable feature. Thanks for taking the time to review this!

@scarroll32
Copy link

+1 to have this merged

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 this pull request may close these issues.

2 participants