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

chore: cleanup whenCurrentJobsFinished #1542

Merged
merged 1 commit into from Nov 8, 2019

Conversation

gabegorelick
Copy link
Contributor

Cleanup and document whenCurrentJobsFinished. And since it's now official API, add some tests for it.

@@ -1174,49 +1174,20 @@ Queue.prototype.clean = function(grace, type, limit) {
* @returns {Promise}
*/
Queue.prototype.whenCurrentJobsFinished = function() {
return new Promise((resolve, reject) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing the unnecessary new Promise.

this.bclient.once('end', function(){
console.error('ENDED!');
setTimeout(function(){
this.bclient.connect();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed all this commented code.

Cleanup and document whenCurrentJobsFinished. And since it's now
official API, add some tests for it.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.005%) to 93.745% when pulling d13d99d on gabegorelick:whenCurrentJobsFinished into d99f7f6 on OptimalBits:develop.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.005%) to 93.745% when pulling d13d99d on gabegorelick:whenCurrentJobsFinished into d99f7f6 on OptimalBits:develop.

@manast manast merged commit 29d0bae into OptimalBits:develop Nov 8, 2019
@gabegorelick gabegorelick deleted the whenCurrentJobsFinished branch November 8, 2019 22:59
}
*/
//this.bclient.connect();
return Promise.all([this.processing[0]]).then(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

After upgrade to latest version of bull we are seeing that on shutdown, it doesn't properly wait for all jobs to complete.

It seems to me that this is likely the change that caused the problem. Was it intentional to change this to only wait for the first job?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, that seems like a dumb mistake on my part. Thanks for catching it @holm.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix: #1586

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the quick fix!

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.

None yet

4 participants