-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
docs(memorystore): added valkey caching practical source-code samples #9986
base: main
Are you sure you want to change the base?
docs(memorystore): added valkey caching practical source-code samples #9986
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @dackers86 and reviewers,
I'm currently reviewing this pull request and will provide my feedback in a few minutes. In the meantime, I'm providing this summary to help everyone quickly understand the changes and intent.
This pull request, "docs(memorystore): added valkey practical source-code samples", aims to add practical source code samples for Memorystore for Valkey. I've analyzed the changes and here's a breakdown:
Intent: Add new, practical code samples demonstrating various Memorystore for Valkey functionalities.
Changes:
- New README.md (memorystore/valkey/caching/samples/README.md): A new README file was added, providing instructions on prerequisites (Java installation, Memorystore for Valkey instance setup), compiling the code via Maven, and running the sample code. The file is 31 lines long.
- New pom.xml (memorystore/valkey/caching/samples/pom.xml): A new
pom.xml
file was added, defining project dependencies (Jedis, Google Cloud Client Libraries, and JUnit for testing). The file is 100 lines long. - New Java source code files: Four new Java files were added under
memorystore/valkey/caching/samples/src/main/java/samples/
:MemorystoreDeleteItem.java
(40 lines): Demonstrates deleting an item from the cache.MemorystoreReadItem.java
(37 lines): Demonstrates reading an item from the cache.MemorystoreTTLItem.java
(63 lines): Demonstrates caching an item with a Time-To-Live (TTL).MemorystoreWriteItem.java
(33 lines): Demonstrates writing an item to the cache.
Checklist: The pull request includes a checklist confirming adherence to the sample format guide, updates to the README, and successful test and lint execution.
I will be checking the code for correctness, style, and adherence to best practices during my review. I'll also verify that the tests pass and that the code is well-documented.
And now, a little haiku to lighten the mood:
Code flows like a stream,
Tests pass, a joyful green light,
Bugs flee in the night.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pull request adds practical source code samples for Memorystore for Valkey, which is a valuable addition. The samples cover basic operations like writing, reading, deleting, and setting TTL for cached items. The provided README.md
is helpful and provides clear instructions.
Here's a summary of the feedback based on the Google Java Style Guide:
- Use descriptive variable names.
- Ensure consistent spacing and indentation.
- Add Javadoc comments for classes and methods.
- Handle potential exceptions.
- Use try-with-resources for resource management.
@dackers86 you need to make sure that all files have the appropriate Apache 2 header please. |
memorystore/valkey/caching/samples/src/main/java/samples/MemorystoreDeleteItem.java
Outdated
Show resolved
Hide resolved
memorystore/valkey/caching/samples/src/main/java/samples/MemorystoreDeleteItem.java
Outdated
Show resolved
Hide resolved
memorystore/valkey/caching/samples/src/main/java/samples/MemorystoreWriteItem.java
Outdated
Show resolved
Hide resolved
… updated pre-requisite descriptions
…ng up a memorystore instance
Altrernativley, run a local instance through the Valkey CLI | ||
|
||
```bash | ||
valkey-cli |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have we validated this? Valkey-cli (as i understand it is a client only) it can't run a local instance of a server. Maybe what you are looking for is something like this? https://valkey.io/topics/server/ or maybe via docker like this? https://hub.docker.com/r/valkey/valkey/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The link to the valkey-cli can found here.
An update to the documentation to include this link would be useful?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok maybe link directly to valkey-cli cluster create command
https://valkey.io/topics/cluster-tutorial/#create-a-valkey-cluster
?
Description
Fixes #
Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google.
Checklist
pom.xml
parent set to latestshared-configuration
mvn clean verify
requiredmvn -P lint checkstyle:check
requiredmvn -P lint clean compile pmd:cpd-check spotbugs:check
advisory only