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

feat: initial implementation of get and set #3

Merged
merged 11 commits into from
Aug 28, 2024

Conversation

malandis
Copy link
Collaborator

@malandis malandis commented Aug 24, 2024

Adds an initial implementation of the MomentoRedisReactiveClient and integration tests:

  • Adds the class MomentoRedisReactiveClient which implements the Lettuce Reactive Redis command set interface. This is a client side proxy that takes a Momento cache client and cache name as dependencies in the constructor. Implements the get and set commands in the first cut.
  • Adds a narrow interface MomentoRedisReactiveCommands with the specific commands we have implemented. This allows a developer the confidence to only use the commands we have authored.
  • Adds a RedisCodec wrapper RedisCodecByteArrayConverter for Momento: Lettuce is generic over the key and value types, allowing a developer to use whatever they want, provided they pass a RedisCodec in the constructor to marshal/unmarshal the data to ByteBuffer. Because Momento takes byte[] and not ByteBuffer we provide this wrapper for internal convenience.
  • Adds a MomentoLettuceExceptionMapper to convert Momento exceptions back to Lettuce exceptions. The relevant exception type here is the timeout exception.
  • Adds integration tests that can run vs Momento or vs Redis. The BaseTestClass sets up the reactive client, which may be backed by either Lettuce or our Momento implementation. To toggle Redis vs Momento, set REDIS=1 in the environment for Redis. The single test class ScalarCommandTest shows how we have a single set of integration tests that can run.
  • Updates the Makefile to run tests vs Redis, vs Momento, or vs both.
  • Updates GitHub CI to run vs Redis and vs Momento

In a future PR we will update the other commands to throw an exception if not implemented.

This wraps the `RedisCodec` which converts to/from ByteBuffer. Because
the Momento client accepts either byte arrays or Strings, we provide
this wrapper for internal convenience.
We have a real test now!
Also updates the slf4j and logback versions.
Previoulsy gradle was caching test runs that completed. Since we run
the tests with an environment variable to toggle redis vs momento, we
do not want that behavior. We also enable logging.
Turn the redis tests into a single step vs a separate job.
@malandis malandis marked this pull request as ready for review August 26, 2024 20:10

@Override
public Mono<String> asking() {
return null;

Choose a reason for hiding this comment

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

just curious how we decided between returning null and throwing e.g. NotImplementedException? not a blocker for this PR but wondering what your thoughts are.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am going to throw an exception in a future PR. (mentioned at the bottom of the PR description :))

Choose a reason for hiding this comment

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

Awesome, thanks. Sorry I missed that in the description.

@malandis malandis merged commit 6bc6499 into main Aug 28, 2024
2 checks passed
@malandis malandis deleted the feat/initial-implementation branch August 28, 2024 20:00
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