-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
fix: call Queue.isReady() before job promote is called #1619
fix: call Queue.isReady() before job promote is called #1619
Conversation
1 similar comment
I test it at my local machine, and it works well. |
} | ||
}); | ||
return queue.isReady().then(() => | ||
scripts.promote(queue, jobId).then(result => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a return is needed here so that the promise returned by promote is propagated correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @manast , do you mean the following part of the code
... .then(() => scripts.promote() ...)
if yes then in es6 the syntax () => value
is identical to () => { return value }
so it is practically like writing
return queue.isReady().then(() => { return scripts.promote(queue, jobId) }).then() ...
If this is not what you meant please correct me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
however if you would like to use the previous syntax () => {return ...}
(which as i can see is used in this file) then i can edit my pr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no sorry, the code is correct, I did not notice the arrow function.
The issue is described here: #1231