Skip to content

Conversation

@amiryal
Copy link

@amiryal amiryal commented Oct 6, 2014

The default loner() method is accepting *args, but they are not passed along where this method is called.

Note, this pull request includes a test case, but it only covers one code path, whereas I fixed 3 different places.

@amiryal amiryal changed the title Make “loner” implementation comply with its implied specification Make “loner” implementation comply with its specification Oct 6, 2014
@lantins
Copy link
Owner

lantins commented Jan 22, 2015

Long delay!

I'm inclined to merge this and trust your judgment, the problem I have is I'm not familiar with the loner plugin :/

Copy link
Owner

Choose a reason for hiding this comment

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

@amiryal I know its a bit to ask, because it's been a while, but I'd appreciate some input from you so we can see this addressed.

From the "loner" docs it says:

Two jobs of the same class with the same parameters/arguments/workload must produce the same redis_key

Does this not mean the 2nd enqueue should fail? and we should have 2 jobs enqueued total?

@amiryal
Copy link
Author

amiryal commented Jan 25, 2015

@lantins, it’s OK, it took me a little while to regain mental image, too.

Notice how the method LonelyWithArgsJob.loner() is implemented, echoing back the argument given to it, defaulting to false. So, in the test, if the job is submitted with no arguments, it should not be treated as a loner, whereas submitting with an argument of true makes it a loner and rejects the second submission.

BTW, if you run the new test on the code before this patch, the first assertion passes. It’s the other assertion that fails, because it depends on propagation of the *args, which the patch fixes.

Hope this helps.

reist added 2 commits May 4, 2015 16:50
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.

3 participants