Execution of middleware chain using blocks, unit tests, and minor cleanup #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mperham
added a commit
that referenced
this pull request
Feb 5, 2012
Execution of middleware chain using blocks, unit tests, and minor cleanup
|
Nicely done! |
|
Thanks Mike! Looking forward to contributing more to the project! |
ghost
mentioned this pull request
Closed
Closed
jr180180
added a commit
to jr180180/sidekiq
that referenced
this pull request
Mar 17, 2022
…nt listeners Related to: sidekiq#5230 ### Bug sidekiq#1: Multiple event listeners are added to elements When adding event listeners to elements, Javascript will generally prevent duplicating the listener if the callback function is named. Anonymous functions aren't tracked the same way. Javascript will continue to add an event listener every time the polling happens. This commit names those functions where the listeners are added to elements outside of the "page" `div`. The duplicate event listeners caused some odd behavior: 1. Clicking in the "Stop Polling" caused a single GET call to the server for each event listener on the element. This will be problematic if someone leaves the polling on for an extended period of time. 2. It would some times cause the "Stop Polling" functionality to not stop polling. #### How to duplicate: 1. Open the page inspector from the developer tools of your browser. 2. Click on the "event" button next to the "Live Poll" button in the inspector. You'll see that there's only one event listener. 3. After the page does a poll, click on it again and you'll now see two event listeners with the same function. 4. Click on the stop polling button 5. Look at your server logs and you'll see a single request per event listener ### Bug sidekiq#2: Ghost interval continues polling after clicking "Stop Polling" Every time the `addListeners` function was called, it would call the `scheduleLivePoll()` function and setup a new `setTimeout`. Thus, when you clicked on the "Stop Polling" button, it would continue to poll because of a stray `setTimeout`. The only way to stop the polling was to refresh. This commit prevents the function from running if a timeout already exists. #### How to duplicate: 1. All the UI to poll for 30 seconds 2. Clear your server logs 3. Click on "Stop Polling" 4. Watch your server logs and you'll see the same polling request continue to come in every five or so seconds.
mperham
referenced
this pull request
Mar 17, 2022
I find `push` is easier to read this way. push_bulk is quite complex and adds a net-negative layer of abstraction.
mperham
pushed a commit
that referenced
this pull request
Mar 17, 2022
…nt listeners (#5247) Related to: #5230 ### Bug #1: Multiple event listeners are added to elements When adding event listeners to elements, Javascript will generally prevent duplicating the listener if the callback function is named. Anonymous functions aren't tracked the same way. Javascript will continue to add an event listener every time the polling happens. This commit names those functions where the listeners are added to elements outside of the "page" `div`. The duplicate event listeners caused some odd behavior: 1. Clicking in the "Stop Polling" caused a single GET call to the server for each event listener on the element. This will be problematic if someone leaves the polling on for an extended period of time. 2. It would some times cause the "Stop Polling" functionality to not stop polling. #### How to duplicate: 1. Open the page inspector from the developer tools of your browser. 2. Click on the "event" button next to the "Live Poll" button in the inspector. You'll see that there's only one event listener. 3. After the page does a poll, click on it again and you'll now see two event listeners with the same function. 4. Click on the stop polling button 5. Look at your server logs and you'll see a single request per event listener ### Bug #2: Ghost interval continues polling after clicking "Stop Polling" Every time the `addListeners` function was called, it would call the `scheduleLivePoll()` function and setup a new `setTimeout`. Thus, when you clicked on the "Stop Polling" button, it would continue to poll because of a stray `setTimeout`. The only way to stop the polling was to refresh. This commit prevents the function from running if a timeout already exists. #### How to duplicate: 1. All the UI to poll for 30 seconds 2. Clear your server logs 3. Click on "Stop Polling" 4. Watch your server logs and you'll see the same polling request continue to come in every five or so seconds.
jasonpenny
pushed a commit
to Appboy/sidekiq
that referenced
this pull request
May 2, 2024
Remove RTT checking from Sidekiq since we have our own monitoring, th…
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hey Mike,
See the pull request. I normally use rspec for tests so perhaps my minitests aren't as clean as they could be. Hopefully it's good enough to get merged in though.
Thanks!
Ryan