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

docs(memorystore): added valkey caching guide #9987

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

dackers86
Copy link

@dackers86 dackers86 commented Jan 22, 2025

Description

Fixes #

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

Checklist

  • I have followed Sample Format Guide
  • pom.xml parent set to latest shared-configuration
  • Appropriate changes to README are included in PR
  • These samples need a new API enabled in testing projects to pass (let us know which ones)
  • These samples need a new/updated env vars in testing projects set to pass (let us know which ones)
  • Tests pass: mvn clean verify required
  • Lint passes: mvn -P lint checkstyle:check required
  • Static Analysis: mvn -P lint clean compile pmd:cpd-check spotbugs:check advisory only
  • This sample adds a new sample directory, and I updated the CODEOWNERS file with the codeowners for this sample
  • This sample adds a new Product API, and I updated the Blunderbuss issue/PR auto-assigner with the codeowners for this sample
  • Please merge this PR for me once it is approved

@dackers86 dackers86 requested review from yoshi-approver and a team as code owners January 22, 2025 17:19
@product-auto-label product-auto-label bot added samples Issues that are directly related to samples. api: memorystore labels Jan 22, 2025
Copy link
Contributor

@code-review-assist code-review-assist bot left a comment

Choose a reason for hiding this comment

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

Hi team,

I'm currently reviewing this pull request and will provide feedback shortly. In the meantime, here's a summary for you and other reviewers to quickly get up to speed:

This pull request adds a new guide on building a caching service on Google Cloud using Valkey, Spring Boot, and PostgreSQL. The intent is to provide a comprehensive tutorial demonstrating how to leverage Valkey for improved application performance and scalability.

Here's a breakdown of the changes:

  • memorystore/valkey/caching/GUIDE.md: This file contains the main guide document. It's a substantial addition (189 lines) detailing the architecture, step-by-step instructions, code snippets (Java using Jedis client), and best practices for implementing a caching solution with Valkey, Spring Boot, and PostgreSQL. The guide covers creating, reading, and deleting cached items, as well as scaling and optimization considerations.
  • memorystore/valkey/caching/GUIDE2.md: This file also contains a guide document (188 lines) that focuses on the benefits of using Valkey for caching and common use cases. It provides a high-level overview of the technology and its advantages before diving into the implementation details.

The checklist in the pull request description indicates that all necessary tests and linting checks have passed.

I'll provide a more detailed review shortly, but this summary should give you a good overview of the changes.


And now, a little haiku to lighten the mood:

Cache speeds up the flow,
Data dances, light and swift,
Users smile with glee.

Copy link
Contributor

@code-review-assist code-review-assist bot 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 a guide for building a caching service using Valkey, Spring Boot, and PostgreSQL. The guide is generally well-written and provides a good overview of the architecture and implementation steps. However, there are a few areas that could be improved:

  • Duplicate Content: The content in GUIDE2.md appears to be a modified version of GUIDE.md. It's unclear why there are two guides. Consider consolidating them into a single, comprehensive guide.
  • Missing get Route Implementation: The guide mentions a get route for retrieving items, but the implementation is missing from the code snippets. Please include the code for the get route.
  • Inconsistent API Route Descriptions: The guide lists create multiple times and mentions delete twice. Please review and correct the API route descriptions.
  • Missing Diagram: The guide mentions adding a diagram of the architecture, but it's missing. A visual representation would greatly enhance understanding.
  • Typos and Grammar: There are a few typos and grammatical errors throughout the guide. Please proofread and correct them.
  • Code Style: The code snippets could benefit from improved formatting and adherence to Java conventions. For example, using consistent indentation and adding Javadoc comments would improve readability.

Overall, the guide is a valuable addition to the documentation. Addressing the above points would further improve its clarity and usefulness.

@iennae iennae added the kokoro:run Add this label to force Kokoro to re-run the tests. label Jan 22, 2025
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Jan 22, 2025
@dackers86 dackers86 force-pushed the @invertase/memorystore-valkey-caching-guide branch from 29670d5 to 2f2427e Compare January 27, 2025 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: memorystore samples Issues that are directly related to samples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants