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.

Sign up for GitHub

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

Jump to bottom

fix(queue-getters): getting undefined when getting repeat jobs #1395

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(queue-getters): getting undefined when getting repeat jobs #1395

wants to merge 1 commit into from

Conversation

ATheCoder
Copy link
Contributor

This will fix #248

This happened because repeat jobs where actually considered to be delayed jobs when using the queue's getJobs function.

I also added a test for it.

@ATheCoder
Copy link
Contributor Author

@manast this post in #248 is exactly the reproduction of this issue.

manast
manast reviewed Aug 29, 2022
View reviewed changes
tests/test_repeat.ts
@@ -1174,4 +1174,16 @@ describe('repeat', function () {
await worker.close();
await queueScheduler.close();
});

it('should get repeatable jobs using the getJobs method of the queue', async function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this test not already passing before this PR?

@manast
Copy link
Contributor

manast commented Aug 29, 2022

@ATheCoder thanks for the PR.

The thing is that "delayed" jobs are not necessary repeat jobs, only the next iteration of the repeat job will be a delayed job. So this fix is not really semantically correct. We have a getter that returns the "meta" part of the repeatable jobs though.
In my opinion what this PR solves could be actually be more correctly resolved by better documentation...

@ATheCoder
Copy link
Contributor Author

@manast I totally agree. However, using the getRepeatableJobs method will only give the meta data of repeatable jobs which is not enough for some use cases where it is important to have all the data.

Do you have any ideas as to what would be a good fix that I can implement?

@ATheCoder
Copy link
Contributor Author

I also tried using getRepeatableJobs along with getJob. But, that did not work either. Because the id field was missing on the metadata provided by getRepeatableJobs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@manast manast manast left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Undefined/empty jobs
2 participants
@ATheCoder @manast