Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Forking worker #13

Merged
merged 4 commits into from Dec 28, 2012

Conversation

Projects
None yet
2 participants
Contributor

danielfarrell commented Dec 23, 2012

Ok, this seems to be working fine here. I added a test for the forking. I think all the rest of the behavior is tested with the simple worker. If you think it should test more within the context of the worker let me know and I'll build that out some more.

Owner

nesquena commented Dec 23, 2012

Looks pretty good, can you add at least one end-to-end test that verifies that jobs are being processed successfully similar to the way we test jobs processed in simple worker

Contributor

danielfarrell commented Dec 23, 2012

Ok, I'll add one. I had tried to run the whole test suite on there but the counting and output capturing doesn't work when you fork. I'll come up with a solution for that though.

Owner

nesquena commented Dec 24, 2012

Great, look forward to having the forking worker available, thanks for working on it.

Contributor

danielfarrell commented Dec 26, 2012

Ok, there are some tests in there for the forking worker adapted from the ThreadsOnFork worker. These changes seems to make one of the "for prepare method" tests fail for the ThreadsOnFork worker, and I'm drawing a blank on why that would be. It seems to be a different one each time, which makes it more confusing. Any thoughts on that would be welcome.

Owner

nesquena commented Dec 26, 2012

Thanks Daniel, I will take a look and see if I can get it to pass on my end. Look forward to merging this in soon.

Owner

nesquena commented Dec 28, 2012

@danielfarrell OK fixed the failing tests, just a simple issue with the logger being defined elsewhere. I cleared the logger at the beginning of those tests.

Owner

nesquena commented Dec 28, 2012

Looks good, unless you see any reason not to, I will merge soon.

Contributor

danielfarrell commented Dec 28, 2012

Cool, thanks for finding that. I knew it had to be something little, I just couldn't find it. I'm good with merging whenever you are.

@nesquena nesquena merged commit 1c57ee2 into nesquena:master Dec 28, 2012

1 check failed

default The Travis build failed
Details
Owner

nesquena commented Dec 28, 2012

Ok merged, released new gem version with forking included, updated wiki https://github.com/nesquena/backburner/wiki/Forking-worker and the readme to include information on the new worker. Thanks Daniel, much appreciated.

@danielfarrell danielfarrell deleted the danielfarrell:forking-worker branch Dec 28, 2012

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