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

Iss 57 implement RANDOMKEY command #94

Merged
merged 6 commits into from
Aug 17, 2024
Merged

Conversation

osteensco
Copy link
Contributor

addresses #57

  • adds randomkey command, Randomkey field to HandlerFuncParams, and randomKey method to keyspace
  • adds Randomkey method to generic api

while my tests pass, I'm not 100% sure if adding a method to echovault/keyspace.go was the ideal approach so let me know if I need to adjust anything.

Copy link
Collaborator

@kelvinmwinuka kelvinmwinuka left a comment

Choose a reason for hiding this comment

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

Hi @osteensco

This is a great PR! I like the approach of adding a randomKey method to keyspace. That's the approach I would've gone for as well.

I made a slight change to your PR. For the RANDOMKEY test, I set up some keys before executing the test. Initially, the test relied on the keys set up by other tests. The problem with this is that it makes the test flaky. Due to the test running in parallel to the other tests in the suite, there's no guarantee that the RANDOMKEY test will run after the other tests. This was leading to an issue where the RANDOMKEY test would periodically fail when no keys were found.

Other than that, it looks good, and I will approve it.

@kelvinmwinuka kelvinmwinuka merged commit 16a4e02 into EchoVault:main Aug 17, 2024
1 check passed
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