-
Notifications
You must be signed in to change notification settings - Fork 11
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
feat: return task id at enqueue and dequeue #654
base: main
Are you sure you want to change the base?
Conversation
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 48654ec:
|
Feat/return task ids
Feat/return task ids
Hey @manuman94, thanks for your contribution. Could you elaborate a little bit more on your use case and how those changes solve your problem? |
Hi! Yes! We are currently developing a Jasmine reporter that sends information to a custom made API (spec information, http requests done, screenshots). Before using queue-promise, this API calls were blocking the actual tests, now using queue-promise we are parallelizing those calls. By getting the task ID from outside the queue we know which async tasks are already pending and which of them are completed. We show information about that while the process is being executed. If one task fail, we know which task is it, and show information about what are we sending and the error returned by the server. That's our use case, but we find useful to keep control of the status of the tasks queued in the promise queue. Thank you, greetings. Side note: sorry about closing pull request, Github doesn't ask for confirmation in any of their actions. I'm very sorry about the confusion. (I'm writing this in my mobile phone, where is easy to miss click buttons) |
Hi, is there any problem with the PR? I can solve whatever thing you need. Greetings. |
Hey @manuman94, first of all thanks for your contribution! I am not sure if this should be a part of this repository. I would recommend you create your own ID/progress system. Maybe something similar to the code below would work for your case: // Simulate a job
function job(shouldSuccess = true) {
return new Promise((resolve, reject) => {
if (shouldSuccess) {
setTimeout(() => resolve(`Success`), 500)
} else {
setTimeout(() => reject(`Failure`), 500)
}
});
}
// Wrap the job with an ID
function executeJob(id, callback) {
return async () => {
try {
const response = await callback();
return { success: true, id, response }
} catch (error) {
// Maybe you can even re-throw here this object literal to keep the
// ID and be able to handle errors in the reject event listener
return { success: false, id, response: error }
}
}
}
const queue = new Queue();
queue.on("resolve", ({ success, id, response }) => {
if (success) {
console.debug(`Resolved task with id ${id}`);
} else {
console.error(`Rejected task with id ${id}`);
}
});
queue.enqueue(executeJob('job-1', () => job(true)));
queue.enqueue(executeJob('job-2', () => job(true)));
queue.enqueue(executeJob('job-3', () => job(true)));
queue.enqueue(executeJob('job-4', () => job(false))); Output
My main concern is the inconsistent API design between the methods and the equivalent events. The enqueue method returns the enqueued tasks ID, but the information is lost when calling dequeue. One has to implement an event listener for the dequeue event and implement his own logic for matching the tasks and dequeued tasks IDs. Another concern that I have is the fact that Using the code snippet I posted above, the task ID is predictable and can be easily associated with the task itself. Let me know your thoughts on this. |
Hi @Bartozzz , thanks for your response! I appreciate your purpose, but my thought is that the queue library should be self sufficient and keep control of the status of the async tasks from outside is something inherent to the library functions. Maybe my approach is not the best, as I agree with all your concerns. But I think the library should offer a way to keep tracking of the promises queued. But that's my opinion! I find your comments correct, so I understand this library is going to another way. Thank you so much for answering and for this awesome library. Greetings. |
Hi! I needed the task ID from outside the queue to keep control of tasks sent to the queue.
This is my approach to get it.
Thanks!