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

Redis::zRangeByScore() expects at most 4 parameters, 6 given #8

Open
nhnFreespirit opened this issue Feb 11, 2013 · 7 comments · May be fixed by #21
Open

Redis::zRangeByScore() expects at most 4 parameters, 6 given #8

nhnFreespirit opened this issue Feb 11, 2013 · 7 comments · May be fixed by #21

Comments

@nhnFreespirit
Copy link

Crashes for me with error in subj. when trying to start worker.

Changing /lib/ResqueScheduler.php line 226 to

$items = Resque::redis()->zrangebyscore('delayed_queue_schedule', '-inf', $at, array("limit" => array(0, 1)));

fixes it for me.

This also better corresponds to the docs in https://github.com/nicolasff/phpredis#zrangebyscore-zrevrangebyscore

evertharmeling pushed a commit to evertharmeling/php-resque-scheduler that referenced this issue Feb 27, 2013
@danhunsaker
Copy link
Contributor

This bug likely came into being due to differences between how Redisent handles this command versus the phpredis extension. The switch from Redisent to Credis in PHP-Resque also added phpredis support (by virtue of Credis supporting the extension, where Redisent does not), and therefore all of the extension's quirks with respect to how these commands fire off.

The above fix adjusts for this change in syntax, as does the referencing pull request.

@chrisboulton
Copy link
Owner

Thanks - I was thinking the same. I've got to cut an updated build of both php-resque and php-resque-scheduler, which I'll hopefully get around to this week. I'll ensure both are fully compatible with each other when I do.

@danhunsaker
Copy link
Contributor

I'll be doing what I can to help, as I intend to use both in a presentation in early May. :)

@oscherler
Copy link

So the bug is still there in colinmollenhour/credis, but nobody reposrts it and the only issue that tracks this is closed?

oscherler referenced this issue in michelsalib/BCCResqueBundle Jun 24, 2014
@javaguirre
Copy link

I don't know if it's related but I've been debugging a problem with this, in this line of php-resque-scheduler, zrangebyscore.

https://github.com/chrisboulton/php-resque-scheduler/blob/master/lib/ResqueScheduler.php#L228

$items = Resque::redis()->zrangebyscore('delayed_queue_schedule', '-inf', $at, array('limit' => array(0, 1)));

Removing the array('limit'...) solved my issues, that's not working for me, It also solved a problem I had with a string conversion Warning, maybe It's because my Redis version(redis-cli 2.8.17).

$items = Resque::redis()->zrangebyscore('delayed_queue_schedule', '-inf', $at);

I didn't work on getting only one element (replacing the limit for something solving the problem), but right now this is fine for me.

@danhunsaker
Copy link
Contributor

Just commented on the BCC bundle issue you cross-posted to, but basically there's a way to fix this that either involves Credis getting fixed (seems unlikely at this ponti) or someone building a PR to check whether the PHP Redis extension is enabled, and choosing how to run this line accordingly (much more likely to be merged). It's pretty basic (in principle, anyway), but I barely have time to comment about it at the moment, so please feel free to write one up for us!

@javaguirre
Copy link

Thanks, sorry for the cross-posting, but I thought It was relevant and didn't want to add just a link.

Your comment was very insightful, I gotta think about what's the best solution and spend some time on it.

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 a pull request may close this issue.

5 participants