Ajaxify retry and remove links in Failure queue. Prevent expensive p… #90
Conversation
|
I'd love to hear some feedback on this PR. The goal was to remove the expensive cycle of reloading the page for each job that has been removed from the failure queue. Often I've found that this cycle of removing or retrying with a reloading of the page can take much too long on an overloaded queue. Ideally this PR would shorten the cycle of getting bad jobs out of the queue by reducing the long load cycle. |
|
This looks to have failed (only) on Ruby 2.2 to install sqlite3.
|
| @@ -19,7 +19,7 @@ class FailuresControllerTest < ActionController::TestCase | |||
| it "deletes the failure" do | |||
| Resque::Failure.expects(:remove).with('123') | |||
| visit(:destroy, {:id => 123}, :method => :delete) | |||
| assert_redirected_to failures_path | |||
| assert_response(:success, '{sucess: true}') | |||
|
Thanks for your PR! Unfortunately I probably won't have time to try this out properly anytime soon. Could anyone else contributing PR's to this project perhaps give a 👍 ? @mattgibson @kirillplatonov |
knappe
force-pushed
the
Ajaxify-Remove-Retry-Links
branch
from
4ab4d52
to
9b3acc7
Compare
|
One thing of note here. I had some trouble testing this out because mocking out a |
|
That failure is the same on that I ran into. Bundle update fixes it. I'll make a PR later to do a bundle update for master, which should fix it for all PRs. Will take a look at the code properly then. |
|
@knappe The tests should pass now if you rebase on master. |
…ge reloads when removing or retrying links.
knappe
force-pushed
the
Ajaxify-Remove-Retry-Links
branch
from
9b3acc7
to
0a23909
Compare
1 similar comment
|
@mcfiredrill Thanks! Rebase'd. |
|
@mcfiredrill @mattgibson Anyone had a chance to look at this, by chance? |
|
@knappe I haven't actually tried it but the code looks good. |
|
@mcfiredrill @mattgibson Just checking in. It has been month since I last checked. |
|
@mcfiredrill @mattgibson @kirillplatonov PR has been ready since May, any chance I could get someone to take a look at it? |
|
As much as I hate them, a browser driven test is probably needed here since it is Javascript. Must the Javascript be part of the view? Can we not separate out into its own asset perhaps? I'll take a look at what this does for real, and comment on the code directly as well |
…age reloads when removing or retrying links.