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

✨ Add ZMPOP #1189

Closed
wants to merge 1 commit into from
Closed

✨ Add ZMPOP #1189

wants to merge 1 commit into from

Conversation

JerrodCarpenter
Copy link
Contributor

@JerrodCarpenter JerrodCarpenter commented May 1, 2023

I noticed some 7.0 commands were missing and thought it might be fun to add them. I'm not super familiar with the code base standards, so I'm sure I stepped in it with a couple of decisions, happy to change whatever.

Also, and this is embarrassing, but I couldn't figure out how to run tests locally.

Example:

irb(main):001:0> redis = Redis.new(url: "redis://localhost:6379/2")
=> #<Redis client v5.0.6 for redis://localhost:6379/2>
irb(main):002:0> redis.zadd("myzset", [[1, "one"], [2, "two"], [3, "three"]])
=> 3
irb(main):003:0> redis.zmpop("myzset", min_or_max: "MAX", count: 2)
=> ["myzset", [["three", 3.0], ["two", 2.0]]]

@JerrodCarpenter
Copy link
Contributor Author

JerrodCarpenter commented May 2, 2023

Not sure if something's up... but I can't seem to get tests to run locally, they hang indefinitely:

  redis-rb git:(master)  make test
Run options: --seed 4443

# Running:

..................................................................................................................................................................................................................

And:

  redis-rb git:(master)  bundle exec rake test:redis
Run options: --seed 31572

# Running:

...................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................Error running mock server: OpenSSL::SSL::SSLError: SSL_accept returned=1 errno=0 peeraddr=127.0.0.1:61251 state=error: tlsv1 alert unknown ca
/Users/jerrod.carpenter/.asdf/installs/ruby/3.2.0/lib/ruby/3.2.0/openssl/ssl.rb:527:in `accept'
/Users/jerrod.carpenter/.asdf/installs/ruby/3.2.0/lib/ruby/3.2.0/openssl/ssl.rb:527:in `accept'
/Users/jerrod.carpenter/Projects/redis-rb/test/support/redis_mock.rb:46:in `block in run'
/Users/jerrod.carpenter/Projects/redis-rb/test/support/redis_mock.rb:38:in `loop'
/Users/jerrod.carpenter/Projects/redis-rb/test/support/redis_mock.rb:38:in `run'
/Users/jerrod.carpenter/Projects/redis-rb/test/support/redis_mock.rb:30:in `block in start'
..Error running mock server: OpenSSL::SSL::SSLError: SSL_accept returned=1 errno=0 peeraddr=127.0.0.1:61260 state=error: tlsv1 alert unknown ca
/Users/jerrod.carpenter/.asdf/installs/ruby/3.2.0/lib/ruby/3.2.0/openssl/ssl.rb:527:in `accept'
/Users/jerrod.carpenter/.asdf/installs/ruby/3.2.0/lib/ruby/3.2.0/openssl/ssl.rb:527:in `accept'
/Users/jerrod.carpenter/Projects/redis-rb/test/support/redis_mock.rb:46:in `block in run'
/Users/jerrod.carpenter/Projects/redis-rb/test/support/redis_mock.rb:38:in `loop'
/Users/jerrod.carpenter/Projects/redis-rb/test/support/redis_mock.rb:38:in `run'
/Users/jerrod.carpenter/Projects/redis-rb/test/support/redis_mock.rb:30:in `block in start'
................................................

Looking into this openssl thing above.. but figured I'd post in case something was obvious

@casperisfine
Copy link

Not too sure what your local error is, but the CI one is legit:

  1) Error:
TestDistributedCommandsOnSortedSets#test_zmpop:
NoMethodError: undefined method `zmpop' for #<Redis client v5.0.6 for redis://127.0.0.1:6381/15>
    /home/runner/work/redis-rb/redis-rb/test/lint/sorted_sets.rb:484:in `block in test_zmpop'
    /home/runner/work/redis-rb/redis-rb/test/helper.rb:159:in `target_version'
    /home/runner/work/redis-rb/redis-rb/test/lint/sorted_sets.rb:483:in `test_zmpop'

You need to define how this command work on Redis::Distributed (in distributed.rb).

Copy link

@casperisfine casperisfine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for the delay I'm traveling right now.

One small error, other than that LGTM.

lib/redis/distributed.rb Outdated Show resolved Hide resolved
@JerrodCarpenter
Copy link
Contributor Author

Apologies for the delay I'm traveling right now.

One small error, other than that LGTM.

No worries, this is just something I'm doing for fun on my off time. Appreciate the patience.

@JerrodCarpenter
Copy link
Contributor Author

Looking at these CI failures.

@byroot byroot closed this in 5904289 May 16, 2023
@byroot
Copy link
Collaborator

byroot commented May 16, 2023

Merged as 5904289

The failure were because foo and foo2 wouldn't hash to the same node. Fixed by using {1}foo and {1}foo2 instead.

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.

None yet

4 participants