Skip to content

Conversation

dackers86
Copy link

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:23
@product-auto-label product-auto-label bot added samples Issues that are directly related to samples. api: memorystore labels Jan 22, 2025
Copy link

@ghost ghost 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 post my full review in a few minutes. In the meantime, here's a quick summary of the changes for you and other reviewers to get up to speed:

This pull request adds a new demo application that showcases Valkey caching with a Spring Boot application. The application interacts with a PostgreSQL database and uses Valkey as an in-memory cache to improve performance.

Here's a breakdown of the changes:

  • New directory: memorystore/valkey/caching/demo added, containing the complete demo application.
  • README.md: A comprehensive README file was added, detailing how to run the application locally using Docker Compose, and how to deploy it to Google Cloud Run (both with a solo API and a web app + API).
  • .gitignore: A comprehensive .gitignore file was added to the app directory to ignore various build artifacts and IDE-specific files.
  • Dockerfile: A Dockerfile was added to the app directory to build a Docker image for the application.
  • docker-compose.yaml: A docker-compose.yaml file was added to the app directory to simplify the local development setup.
  • init.sql: An init.sql script was added to the app directory to create the necessary database tables.
  • pom.xml: The pom.xml file was updated to include dependencies for Spring Boot, Jedis (Redis client), PostgreSQL JDBC driver, and other necessary libraries.
  • Java files: Several Java files were added or modified, including DataController, HomeController, Item, ItemController, ItemsRepository, JdbcConfig, JedisConfig, and Main, implementing the core logic of the caching demo application.
  • static/script.js: A JavaScript file was added to the static directory to handle the frontend interactions with the API.
  • templates/index.html: An HTML template was added to the templates directory to provide a user interface for the application.
  • sample-data directory: A sample-data directory was added, containing a Dockerfile, docker-compose.yaml, and pom.xml to create sample data for the application.

The checklist in the PR description indicates that all required tests and lint checks have passed.

I'll provide a more detailed review shortly.

And now, a little haiku to brighten your day:

Cache speeds up the code,
Data flows with graceful ease,
Reviews now are fun!

Copy link

@ghost ghost 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 demo application showcasing Valkey caching with Memorystore. The documentation and code are well-structured and cover various aspects of setup and deployment. However, there are a few minor improvements that can enhance clarity and user experience.

I've summarized the Google Java Style Guide elements used in this review below:

  • Javadoc: Ensured Javadoc comments are present for classes and public methods, providing clear descriptions of their purpose and usage.
  • Naming Conventions: Checked for consistent and descriptive names for classes, methods, and variables.
  • Constant Variables: Verified the use of uppercase with underscores for constant variables.
  • Comments: Recommended adding comments to explain complex logic or non-obvious code segments.
  • Error Handling: Suggested improvements for error handling and validation to prevent unexpected behavior.

@telpirion telpirion self-assigned this Mar 24, 2025
@telpirion
Copy link
Contributor

I'm going to close this PR for the following reasons:

  1. This repo is intended for documentation hosted on cloud.google.com. The code in this PR looks like a complete application. While intriguing, I don't think java-docs-samples is the best location for this. I encourage you to consider writing a blog to describe what you've built here.
  2. This PR is too large. Ideally a samples PR is no more than 500 LoC.
  3. This PR has more code -- Terraform, JS -- than just Java.
  4. Also see my feedback to feat(memorystore): added valkey caching practical source-code samples #9986 . This needs tests, region tags, style updates.

@telpirion telpirion closed this Mar 24, 2025
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