worker_pids() does not return correct set of pyres worker pids #122

merged 4 commits into from Jan 22, 2013


None yet

2 participants

lcwill commented Jan 20, 2013

The worker_pids() method executes "ps -A -o pid,command | grep pyres_worker", which potentially returns the pid of the grep process in addition to the worker pids. When there are no workers running, it returns an array with an empty string as its only element. This does not seem to fit the expected behavior of this method. In addition, it should not really be an instance method.

I've fixed the logic for getting the list of worker pids, made it a class method, and added a test case. I've also fixed a small (unrelated) bug in the ResQ.str() method.


It seems the test you've added is not passing. Also, what's the reasoning behind changing the method to be a class method vs object method?

lcwill commented Jan 22, 2013

Thanks for pointing that out - I've fixed the test case and the build looks good now.

The worker_pids() method (similar to all() and find()) does not operate directly on a worker instance and I think it makes more sense as a class method. IMO, any other code that calls this method should not have to first instantiate a worker object, given that the intent of the method is to provide information about all workers and not a specific instance.

@binarydud binarydud merged commit d936b45 into binarydud:master Jan 22, 2013

1 check passed

default The Travis build passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment