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

[5.6] Fix check on empty queue #23757

Merged
merged 1 commit into from Mar 31, 2018

Conversation

Projects
None yet
3 participants
@pdavide
Copy link
Contributor

commented Mar 30, 2018

When using phpredis, blpop returns an empty array if no elements are found in the list.
This can be easily deduced looking at this test.

Fix #23756.

Fixed check on empty queue
When using phpredis, `blpop` returns an empty array if no elements are found in the list.
This can be easily deduced looking at [this test](https://github.com/phpredis/phpredis/blob/300c72510c48e210338826b713f260a4eda8abc7/tests/RedisTest.php#L833).

Fix #23756.

@pdavide pdavide changed the title Fixed check on empty queue [5.6] Fixed check on empty queue Mar 30, 2018

@pdavide pdavide changed the title [5.6] Fixed check on empty queue [5.6] Fix check on empty queue Mar 30, 2018

@tillkruss

This comment has been minimized.

Copy link
Member

commented Mar 31, 2018

I'd rather see the PhpRedisConnection fixed, so we can rely on the return values across drivers.

@pdavide

This comment has been minimized.

Copy link
Contributor Author

commented Mar 31, 2018

Well, I think my simple solution will ensure compatibility across the drivers, as empty return true with both NULL and Array().
Additionally this little bug cause the application log file to grow fast and become unreadable because the queue worker is calling the blockingPop function every time it searches for an available job and the $rawBody[1] expression will raise an error when $rawbody is an empty array.

@taylorotwell taylorotwell merged commit 570b489 into laravel:5.6 Mar 31, 2018

2 checks passed

continuous-integration/styleci/pr The analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@tillkruss

This comment has been minimized.

Copy link
Member

commented Mar 31, 2018

@pdavide: It's only solved in RedisQueue, nowhere else. See #23761.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.