Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat(memorystore): added valkey caching practical source-code samples#9986

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
dackers86 wants to merge28 commits intoGoogleCloudPlatform:main
base:main
Choose a base branch
Loading
frominvertase:@invertase/memorystore-valkey-caching-practical-source-code

Conversation

dackers86
Copy link

@dackers86dackers86 commentedJan 22, 2025
edited
Loading

Description

Fixes #

Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google.

Checklist

  • I have followedSample Format Guide
  • pom.xml parent set to latestshared-configuration
  • Appropriate changes to README are included in PR
  • These samples need a newAPI enabled in testing projects to pass (let us know which ones)
  • These samples need a new/updatedenv vars in testing projects set to pass (let us know which ones)
  • Tests pass:mvn clean verifyrequired
  • Lint passes:mvn -P lint checkstyle:checkrequired
  • Static Analysis:mvn -P lint clean compile pmd:cpd-check spotbugs:checkadvisory only
  • This sample adds a new sample directory, and I updated theCODEOWNERS file with the codeowners for this sample
  • This sample adds a newProduct API, and I updated theBlunderbuss issue/PR auto-assigner with the codeowners for this sample
  • Pleasemerge this PR for me once it is approved

telpirion reacted with eyes emoji
@product-auto-labelproduct-auto-labelbot added the samplesIssues that are directly related to samples. labelJan 22, 2025
Copy link

@ghostghost left a 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 newpom.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 undermemorystore/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.

Copy link

@ghostghost left a 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 providedREADME.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.

@iennae
Copy link
Contributor

@dackers86 you need to make sure that all files have the appropriate Apache 2 header please.

dackers86 reacted with thumbs up emoji

@dackers86dackers86 changed the titledocs(memorystore): added valkey practical source-code samplesdocs(memorystore): added valkey caching practical source-code samplesJan 28, 2025
Altrernativley, run a local instance through the Valkey CLI

```bash
valkey-cli

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/

Copy link
Author

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 foundhere.

An update to the documentation to include this link would be useful?

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
?

@telpiriontelpirion changed the titledocs(memorystore): added valkey caching practical source-code samplesfeat(memorystore): added valkey caching practical source-code samplesMar 24, 2025
@telpirion
Copy link
Contributor

Hello@dackers86 -- this PR needs a bit more work before we can accept it.

Before resubmitting, please be sure to review theGoogle Samples Style Guide and theSAMPLE_FORMAT guide before re-opening. I encourage you to look at other samples in this repository to see how samples are typically formatted and styled.

This PR needs to have the following global issues addressed:

  • Provide region tags for the samples.memorystore is the prefix for this product.
  • Provide samples integration tests.
  • Provide a main method and a sample method in each code file. The main method passes arguments to the sample method.
  • Remove static field declarations in the samples. See previous comment -- pass arguments from main to samples method.
  • Remove unused, private constructors.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@fewtrellfewtrellfewtrell approved these changes

@yoshi-approveryoshi-approverAwaiting requested review from yoshi-approveryoshi-approver is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
api: memorystoresamplesIssues that are directly related to samples.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

7 participants
@dackers86@iennae@telpirion@fewtrell@kweinmeister@cabljac@CorieW

[8]ページ先頭

©2009-2025 Movatter.jp