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.

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

Fix incorrect unpacking of job data #82

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

Conversation

hamishforbes
Copy link

Fix 2 cases where job data fields (state and worker) are passed to unpack even though they are strings.

The first case (state), the variable was unused anyway so I just removed it totally.

@dlecocq
Copy link
Contributor

dlecocq commented Aug 6, 2018

I believe QlessJob:data always returns a table. Still, the tests pass both ways, and test_events.py's TestEvents.test_failed_retries exercises this code.

@hamishforbes
Copy link
Author

hamishforbes commented Aug 6, 2018

Oh I see, sorry that's my bad. I looked at that and though it was indexing the return from data().
Didn't notice that its an argument that filters the return value.

The variable is still unused in the first change (state) but looks like the worker one should be broken.
I'll remove that change

edit: I think actually the errors we were seeing were caused by a too-small Redis memory limit causing evictions in the middle of job processing

@dlecocq
Copy link
Contributor

dlecocq commented Aug 6, 2018

LGTM

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.

2 participants