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

🔥 Feature: Add valkey driver #1538

Merged
merged 17 commits into from
Jan 7, 2025
Merged

Conversation

grivera64
Copy link
Member

@grivera64 grivera64 commented Jan 4, 2025

Description

This PR adds valkey support to Fiber through a storage module.

Fixes #1361

Changes

  • Copies rueidis driver as template to valkey/ (and test cases)
  • Adds valkey-io/valkey-go as a dependency for the storage module
  • Adds workflows for testing
    • Note: This may need some changes as I couldn't run the workflows before making the PR yet.

  • (WIP) Update README.md to refer to Valkey rather than Redis

Summary by CodeRabbit

Release Notes

  • New Features

    • Added Valkey storage implementation for key-value operations.
    • Introduced support for Redis-style connections with TLS configuration.
    • Implemented client-side caching and auto pipelining.
    • Enhanced test certificate generation script to create a new "valkey" certificate.
  • Documentation

    • Created comprehensive README for Valkey storage system.
    • Added installation and usage instructions.
    • Detailed configuration options and default settings.
    • Updated project README to include Valkey storage link with test status badge.
  • Tests

    • Added unit tests and benchmarks for Valkey storage functionalities.

Copy link
Contributor

coderabbitai bot commented Jan 4, 2025

Walkthrough

This pull request introduces a new Valkey storage implementation for the Fiber framework. The changes include adding a new package valkey with comprehensive functionality for key-value storage operations, complete with configuration options, a modified test certificates generation script, and updated documentation. The implementation supports features like TLS configuration, connection management, and basic CRUD operations for key-value pairs.

Changes

File Change Summary
.github/scripts/gen-test-certs.sh Added new certificate generation for "valkey" and current directory output
README.md Added Valkey storage implementation link
valkey/README.md New documentation for Valkey storage implementation
valkey/config.go Defined configuration structure and default settings
valkey/valkey.go Implemented core storage functionality with key-value operations
valkey/valkey_test.go Added comprehensive unit and benchmark tests

Assessment against linked issues

Objective Addressed Explanation
Implement Valkey Storage [#1361]

Suggested Labels

♻ Wait for Response

Poem

🐰 In the realm of bytes and keys so bright,
A Valkey storage dances with delight!
Swift connections, caching with grace,
Fiber's new friend finds its place
Hopping through data, quick and clean! 🚀


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@grivera64 grivera64 self-assigned this Jan 4, 2025
@gaby
Copy link
Member

gaby commented Jan 4, 2025

@grivera64 At this point I would say to use go1.23 for your go.mod

1.24 is coming out soon, so 1.22 will be EOL

@grivera64 grivera64 marked this pull request as ready for review January 4, 2025 07:22
@grivera64 grivera64 requested a review from a team as a code owner January 4, 2025 07:22
@grivera64 grivera64 requested review from gaby, sixcolors, ReneWerner87 and efectn and removed request for a team January 4, 2025 07:22
@grivera64
Copy link
Member Author

@grivera64 At this point I would say to use go1.23 for your go.mod

1.24 is coming out soon, so 1.22 will be EOL

Sounds good! I've update the Go version to 1.23.x.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (14)
valkey/valkey.go (5)

11-16: Consider potential concurrency impact of using a global variable cacheTTL.
Having a package-level variable (cacheTTL) that is shared across all Storage instances may lead to race conditions if cfg.CacheTTL is reconfigured in concurrent usage.

- var cacheTTL = time.Second

// Instead, store the TTL in the Storage struct to avoid concurrency issues.
 type Storage struct {
-  db valkey.Client
+  db        valkey.Client
+  cacheTTL  time.Duration
 }

18-37: Avoid panicking on configuration or parsing failures.
Panicking in production code can crash the entire application. It’s generally preferable to return an error and let callers handle it.

- // This will panic if parsing URL fails
- options := valkey.MustParseURL(cfg.URL)
+ options, err := valkey.ParseURL(cfg.URL)
+ if err != nil {
+   return nil, fmt.Errorf("parse URL failed: %w", err)
+ }

49-69: Consider handling client creation errors without panicking.
Similarly, in case valkey.NewClient fails, it may be more robust to wrap and return the error instead of throwing a panic.

 if err != nil {
-  panic(err)
+  return nil, fmt.Errorf("valkey.NewClient failed: %w", err)
 }

90-99: Return specific errors or messages for invalid keys.
Returning nil, nil obscures the reason for an empty response. Consider returning a distinct error or log message if the input key is empty.


71-74: Optional: Provide graceful error handling for ping failures.
By using panic(err), the entire application stops if the connection check fails. For better resiliency, you could return an error or fallback logic.

valkey/config.go (2)

104-124: Ensure references to Redis are changed to Valkey if needed.
The docstrings mention "Redis" in a few places. Double-check references to align with the actual underlying technology (Valkey).


127-198: Check concurrency usage for updating ConfigDefault.
Although ConfigDefault is only read here, if any future code modifies it, concurrency issues could arise. Consider making ConfigDefault unexported or returning a copy.

valkey/valkey_test.go (2)

12-14: Consider grouping test stores with different configs.
Relying on the same testStore for multiple tests can lead to inter-test interference if not carefully reset. This might complicate test maintenance.


63-64: Add an assertion verifying expiration success.
After sleeping 1.1 seconds, consider re-checking the key here to confirm it has indeed expired, rather than in a separate test. This could strengthen the test.

.github/scripts/gen-test-certs.sh (1)

59-63: Generate distinct certificate names if valkey serves a different domain.
If you plan to run Valkey tests concurrently with Redis tests, issuing certificates with the same CN ("localhost") might cause collisions.

valkey/README.md (4)

12-12: Hyphenate compound adjectives for clarity.
Suggest using "auto-pipelining" and "client-side caching" for better readability.

- A fast Valkey Storage that does auto pipelining and supports client side caching.
+ A fast Valkey Storage that does auto-pipelining and supports client-side caching.
🧰 Tools
🪛 LanguageTool

[uncategorized] ~12-~12: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...inter) A fast Valkey Storage that does auto pipelining and supports client side caching. [valk...

(AUTO_HYPHEN)


[uncategorized] ~12-~12: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... that does auto pipelining and supports client side caching. [valkey-io/valkey](https://git...

(EN_COMPOUND_ADJECTIVE_INTERNAL)


49-49: Proposal: Remove redundant article in “the one of the following.”

- You can use the one of the following options
+ You can use one of the following options
🧰 Tools
🪛 LanguageTool

[misspelling] ~49-~49: Make sure that ‘the one of’ is correct and that ‘one’ is a pronoun. Possibly, the ‘the’ is unnecessary or ‘of’ is better expressed with a preposition such as ‘about’ or ‘in’.
Context: ...ofiber/storage/valkey" ``` You can use the one of the following options to create a Rueid...

(THE_ONE_OF_DT)


77-210: Address Markdownlint warnings about hard tabs (MD010).
Replace hard tabs with spaces for better consistency and compliance with linting rules.

🧰 Tools
🪛 Markdownlint (0.37.0)

77-77: Column: 1
Hard tabs

(MD010, no-hard-tabs)


78-78: Column: 1
Hard tabs

(MD010, no-hard-tabs)


81-81: Column: 1
Hard tabs

(MD010, no-hard-tabs)


82-82: Column: 1
Hard tabs

(MD010, no-hard-tabs)


83-83: Column: 1
Hard tabs

(MD010, no-hard-tabs)


98-98: Column: 1
Hard tabs

(MD010, no-hard-tabs)


99-99: Column: 1
Hard tabs

(MD010, no-hard-tabs)


100-100: Column: 1
Hard tabs

(MD010, no-hard-tabs)


101-101: Column: 1
Hard tabs

(MD010, no-hard-tabs)


103-103: Column: 1
Hard tabs

(MD010, no-hard-tabs)


104-104: Column: 1
Hard tabs

(MD010, no-hard-tabs)


105-105: Column: 1
Hard tabs

(MD010, no-hard-tabs)


106-106: Column: 1
Hard tabs

(MD010, no-hard-tabs)


108-108: Column: 1
Hard tabs

(MD010, no-hard-tabs)


109-109: Column: 1
Hard tabs

(MD010, no-hard-tabs)


110-110: Column: 1
Hard tabs

(MD010, no-hard-tabs)


111-111: Column: 1
Hard tabs

(MD010, no-hard-tabs)


113-113: Column: 1
Hard tabs

(MD010, no-hard-tabs)


114-114: Column: 1
Hard tabs

(MD010, no-hard-tabs)


115-115: Column: 1
Hard tabs

(MD010, no-hard-tabs)


116-116: Column: 1
Hard tabs

(MD010, no-hard-tabs)


117-117: Column: 1
Hard tabs

(MD010, no-hard-tabs)


119-119: Column: 1
Hard tabs

(MD010, no-hard-tabs)


120-120: Column: 1
Hard tabs

(MD010, no-hard-tabs)


121-121: Column: 1
Hard tabs

(MD010, no-hard-tabs)


122-122: Column: 1
Hard tabs

(MD010, no-hard-tabs)


124-124: Column: 1
Hard tabs

(MD010, no-hard-tabs)


125-125: Column: 1
Hard tabs

(MD010, no-hard-tabs)


126-126: Column: 1
Hard tabs

(MD010, no-hard-tabs)


127-127: Column: 1
Hard tabs

(MD010, no-hard-tabs)


129-129: Column: 1
Hard tabs

(MD010, no-hard-tabs)


130-130: Column: 1
Hard tabs

(MD010, no-hard-tabs)


131-131: Column: 1
Hard tabs

(MD010, no-hard-tabs)


132-132: Column: 1
Hard tabs

(MD010, no-hard-tabs)


134-134: Column: 1
Hard tabs

(MD010, no-hard-tabs)


135-135: Column: 1
Hard tabs

(MD010, no-hard-tabs)


136-136: Column: 1
Hard tabs

(MD010, no-hard-tabs)


137-137: Column: 1
Hard tabs

(MD010, no-hard-tabs)


139-139: Column: 1
Hard tabs

(MD010, no-hard-tabs)


140-140: Column: 1
Hard tabs

(MD010, no-hard-tabs)


141-141: Column: 1
Hard tabs

(MD010, no-hard-tabs)


142-142: Column: 1
Hard tabs

(MD010, no-hard-tabs)


144-144: Column: 1
Hard tabs

(MD010, no-hard-tabs)


145-145: Column: 1
Hard tabs

(MD010, no-hard-tabs)


146-146: Column: 1
Hard tabs

(MD010, no-hard-tabs)


147-147: Column: 1
Hard tabs

(MD010, no-hard-tabs)


149-149: Column: 1
Hard tabs

(MD010, no-hard-tabs)


150-150: Column: 1
Hard tabs

(MD010, no-hard-tabs)


151-151: Column: 1
Hard tabs

(MD010, no-hard-tabs)


152-152: Column: 1
Hard tabs

(MD010, no-hard-tabs)


154-154: Column: 1
Hard tabs

(MD010, no-hard-tabs)


155-155: Column: 1
Hard tabs

(MD010, no-hard-tabs)


156-156: Column: 1
Hard tabs

(MD010, no-hard-tabs)


157-157: Column: 1
Hard tabs

(MD010, no-hard-tabs)


159-159: Column: 1
Hard tabs

(MD010, no-hard-tabs)


160-160: Column: 1
Hard tabs

(MD010, no-hard-tabs)


161-161: Column: 1
Hard tabs

(MD010, no-hard-tabs)


162-162: Column: 1
Hard tabs

(MD010, no-hard-tabs)


164-164: Column: 1
Hard tabs

(MD010, no-hard-tabs)


165-165: Column: 1
Hard tabs

(MD010, no-hard-tabs)


166-166: Column: 1
Hard tabs

(MD010, no-hard-tabs)


167-167: Column: 1
Hard tabs

(MD010, no-hard-tabs)


169-169: Column: 1
Hard tabs

(MD010, no-hard-tabs)


170-170: Column: 1
Hard tabs

(MD010, no-hard-tabs)


171-171: Column: 1
Hard tabs

(MD010, no-hard-tabs)


172-172: Column: 1
Hard tabs

(MD010, no-hard-tabs)


174-174: Column: 1
Hard tabs

(MD010, no-hard-tabs)


175-175: Column: 1
Hard tabs

(MD010, no-hard-tabs)


176-176: Column: 1
Hard tabs

(MD010, no-hard-tabs)


177-177: Column: 1
Hard tabs

(MD010, no-hard-tabs)


179-179: Column: 1
Hard tabs

(MD010, no-hard-tabs)


180-180: Column: 1
Hard tabs

(MD010, no-hard-tabs)


181-181: Column: 1
Hard tabs

(MD010, no-hard-tabs)


182-182: Column: 1
Hard tabs

(MD010, no-hard-tabs)


184-184: Column: 1
Hard tabs

(MD010, no-hard-tabs)


185-185: Column: 1
Hard tabs

(MD010, no-hard-tabs)


186-186: Column: 1
Hard tabs

(MD010, no-hard-tabs)


187-187: Column: 1
Hard tabs

(MD010, no-hard-tabs)


194-194: Column: 1
Hard tabs

(MD010, no-hard-tabs)


195-195: Column: 1
Hard tabs

(MD010, no-hard-tabs)


196-196: Column: 1
Hard tabs

(MD010, no-hard-tabs)


197-197: Column: 1
Hard tabs

(MD010, no-hard-tabs)


198-198: Column: 1
Hard tabs

(MD010, no-hard-tabs)


199-199: Column: 1
Hard tabs

(MD010, no-hard-tabs)


200-200: Column: 1
Hard tabs

(MD010, no-hard-tabs)


201-201: Column: 1
Hard tabs

(MD010, no-hard-tabs)


202-202: Column: 1
Hard tabs

(MD010, no-hard-tabs)


203-203: Column: 1
Hard tabs

(MD010, no-hard-tabs)


204-204: Column: 1
Hard tabs

(MD010, no-hard-tabs)


205-205: Column: 1
Hard tabs

(MD010, no-hard-tabs)


206-206: Column: 1
Hard tabs

(MD010, no-hard-tabs)


207-207: Column: 1
Hard tabs

(MD010, no-hard-tabs)


208-208: Column: 1
Hard tabs

(MD010, no-hard-tabs)


209-209: Column: 1
Hard tabs

(MD010, no-hard-tabs)


210-210: Column: 1
Hard tabs

(MD010, no-hard-tabs)


14-14: Avoid emphasis as a heading.
Markdownlint (MD036) flags the usage of **Note:** as a heading. Consider a heading instead for better structure.

🧰 Tools
🪛 Markdownlint (0.37.0)

14-14: null
Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a0d6e6d and 9013b62.

⛔ Files ignored due to path filters (5)
  • .github/release-drafter-valkey.yml is excluded by !**/*.yml
  • .github/workflows/release-drafter-valkey.yml is excluded by !**/*.yml
  • .github/workflows/test-valkey.yml is excluded by !**/*.yml
  • valkey/go.mod is excluded by !**/*.mod
  • valkey/go.sum is excluded by !**/*.sum, !**/*.sum
📒 Files selected for processing (6)
  • .github/scripts/gen-test-certs.sh (1 hunks)
  • README.md (1 hunks)
  • valkey/README.md (1 hunks)
  • valkey/config.go (1 hunks)
  • valkey/valkey.go (1 hunks)
  • valkey/valkey_test.go (1 hunks)
🧰 Additional context used
🪛 Markdownlint (0.37.0)
README.md

78-78: null
Images should have alternate text (alt text)

(MD045, no-alt-text)

valkey/README.md

77-77: Column: 1
Hard tabs

(MD010, no-hard-tabs)


78-78: Column: 1
Hard tabs

(MD010, no-hard-tabs)


81-81: Column: 1
Hard tabs

(MD010, no-hard-tabs)


82-82: Column: 1
Hard tabs

(MD010, no-hard-tabs)


83-83: Column: 1
Hard tabs

(MD010, no-hard-tabs)


98-98: Column: 1
Hard tabs

(MD010, no-hard-tabs)


99-99: Column: 1
Hard tabs

(MD010, no-hard-tabs)


100-100: Column: 1
Hard tabs

(MD010, no-hard-tabs)


101-101: Column: 1
Hard tabs

(MD010, no-hard-tabs)


103-103: Column: 1
Hard tabs

(MD010, no-hard-tabs)


104-104: Column: 1
Hard tabs

(MD010, no-hard-tabs)


105-105: Column: 1
Hard tabs

(MD010, no-hard-tabs)


106-106: Column: 1
Hard tabs

(MD010, no-hard-tabs)


108-108: Column: 1
Hard tabs

(MD010, no-hard-tabs)


109-109: Column: 1
Hard tabs

(MD010, no-hard-tabs)


110-110: Column: 1
Hard tabs

(MD010, no-hard-tabs)


111-111: Column: 1
Hard tabs

(MD010, no-hard-tabs)


113-113: Column: 1
Hard tabs

(MD010, no-hard-tabs)


114-114: Column: 1
Hard tabs

(MD010, no-hard-tabs)


115-115: Column: 1
Hard tabs

(MD010, no-hard-tabs)


116-116: Column: 1
Hard tabs

(MD010, no-hard-tabs)


117-117: Column: 1
Hard tabs

(MD010, no-hard-tabs)


119-119: Column: 1
Hard tabs

(MD010, no-hard-tabs)


120-120: Column: 1
Hard tabs

(MD010, no-hard-tabs)


121-121: Column: 1
Hard tabs

(MD010, no-hard-tabs)


122-122: Column: 1
Hard tabs

(MD010, no-hard-tabs)


124-124: Column: 1
Hard tabs

(MD010, no-hard-tabs)


125-125: Column: 1
Hard tabs

(MD010, no-hard-tabs)


126-126: Column: 1
Hard tabs

(MD010, no-hard-tabs)


127-127: Column: 1
Hard tabs

(MD010, no-hard-tabs)


129-129: Column: 1
Hard tabs

(MD010, no-hard-tabs)


130-130: Column: 1
Hard tabs

(MD010, no-hard-tabs)


131-131: Column: 1
Hard tabs

(MD010, no-hard-tabs)


132-132: Column: 1
Hard tabs

(MD010, no-hard-tabs)


134-134: Column: 1
Hard tabs

(MD010, no-hard-tabs)


135-135: Column: 1
Hard tabs

(MD010, no-hard-tabs)


136-136: Column: 1
Hard tabs

(MD010, no-hard-tabs)


137-137: Column: 1
Hard tabs

(MD010, no-hard-tabs)


139-139: Column: 1
Hard tabs

(MD010, no-hard-tabs)


140-140: Column: 1
Hard tabs

(MD010, no-hard-tabs)


141-141: Column: 1
Hard tabs

(MD010, no-hard-tabs)


142-142: Column: 1
Hard tabs

(MD010, no-hard-tabs)


144-144: Column: 1
Hard tabs

(MD010, no-hard-tabs)


145-145: Column: 1
Hard tabs

(MD010, no-hard-tabs)


146-146: Column: 1
Hard tabs

(MD010, no-hard-tabs)


147-147: Column: 1
Hard tabs

(MD010, no-hard-tabs)


149-149: Column: 1
Hard tabs

(MD010, no-hard-tabs)


150-150: Column: 1
Hard tabs

(MD010, no-hard-tabs)


151-151: Column: 1
Hard tabs

(MD010, no-hard-tabs)


152-152: Column: 1
Hard tabs

(MD010, no-hard-tabs)


154-154: Column: 1
Hard tabs

(MD010, no-hard-tabs)


155-155: Column: 1
Hard tabs

(MD010, no-hard-tabs)


156-156: Column: 1
Hard tabs

(MD010, no-hard-tabs)


157-157: Column: 1
Hard tabs

(MD010, no-hard-tabs)


159-159: Column: 1
Hard tabs

(MD010, no-hard-tabs)


160-160: Column: 1
Hard tabs

(MD010, no-hard-tabs)


161-161: Column: 1
Hard tabs

(MD010, no-hard-tabs)


162-162: Column: 1
Hard tabs

(MD010, no-hard-tabs)


164-164: Column: 1
Hard tabs

(MD010, no-hard-tabs)


165-165: Column: 1
Hard tabs

(MD010, no-hard-tabs)


166-166: Column: 1
Hard tabs

(MD010, no-hard-tabs)


167-167: Column: 1
Hard tabs

(MD010, no-hard-tabs)


169-169: Column: 1
Hard tabs

(MD010, no-hard-tabs)


170-170: Column: 1
Hard tabs

(MD010, no-hard-tabs)


171-171: Column: 1
Hard tabs

(MD010, no-hard-tabs)


172-172: Column: 1
Hard tabs

(MD010, no-hard-tabs)


174-174: Column: 1
Hard tabs

(MD010, no-hard-tabs)


175-175: Column: 1
Hard tabs

(MD010, no-hard-tabs)


176-176: Column: 1
Hard tabs

(MD010, no-hard-tabs)


177-177: Column: 1
Hard tabs

(MD010, no-hard-tabs)


179-179: Column: 1
Hard tabs

(MD010, no-hard-tabs)


180-180: Column: 1
Hard tabs

(MD010, no-hard-tabs)


181-181: Column: 1
Hard tabs

(MD010, no-hard-tabs)


182-182: Column: 1
Hard tabs

(MD010, no-hard-tabs)


184-184: Column: 1
Hard tabs

(MD010, no-hard-tabs)


185-185: Column: 1
Hard tabs

(MD010, no-hard-tabs)


186-186: Column: 1
Hard tabs

(MD010, no-hard-tabs)


187-187: Column: 1
Hard tabs

(MD010, no-hard-tabs)


194-194: Column: 1
Hard tabs

(MD010, no-hard-tabs)


195-195: Column: 1
Hard tabs

(MD010, no-hard-tabs)


196-196: Column: 1
Hard tabs

(MD010, no-hard-tabs)


197-197: Column: 1
Hard tabs

(MD010, no-hard-tabs)


198-198: Column: 1
Hard tabs

(MD010, no-hard-tabs)


199-199: Column: 1
Hard tabs

(MD010, no-hard-tabs)


200-200: Column: 1
Hard tabs

(MD010, no-hard-tabs)


201-201: Column: 1
Hard tabs

(MD010, no-hard-tabs)


202-202: Column: 1
Hard tabs

(MD010, no-hard-tabs)


203-203: Column: 1
Hard tabs

(MD010, no-hard-tabs)


204-204: Column: 1
Hard tabs

(MD010, no-hard-tabs)


205-205: Column: 1
Hard tabs

(MD010, no-hard-tabs)


206-206: Column: 1
Hard tabs

(MD010, no-hard-tabs)


207-207: Column: 1
Hard tabs

(MD010, no-hard-tabs)


208-208: Column: 1
Hard tabs

(MD010, no-hard-tabs)


209-209: Column: 1
Hard tabs

(MD010, no-hard-tabs)


210-210: Column: 1
Hard tabs

(MD010, no-hard-tabs)


14-14: null
Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

🪛 LanguageTool
valkey/README.md

[style] ~6-~6: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 1157 characters long)
Context: ...b/v/tag/gofiber/storage?filter=valkey*) Discord Test ![Security](https://img.shields.io/github...

(EN_EXCESSIVE_EXCLAMATION)


[uncategorized] ~12-~12: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...inter) A fast Valkey Storage that does auto pipelining and supports client side caching. [valk...

(AUTO_HYPHEN)


[uncategorized] ~12-~12: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... that does auto pipelining and supports client side caching. [valkey-io/valkey](https://git...

(EN_COMPOUND_ADJECTIVE_INTERNAL)


[misspelling] ~49-~49: Make sure that ‘the one of’ is correct and that ‘one’ is a pronoun. Possibly, the ‘the’ is unnecessary or ‘of’ is better expressed with a preposition such as ‘about’ or ‘in’.
Context: ...ofiber/storage/valkey" ``` You can use the one of the following options to create a Rueid...

(THE_ONE_OF_DT)

🔇 Additional comments (2)
valkey/valkey.go (1)

102-111: Review silent behavior for invalid inputs.
The function silently succeeds when given an empty key or val. Verify whether this is intended. A possible improvement: return an error that indicates invalid input.

valkey/valkey_test.go (1)

126-137: Ensure certificate paths exist or make them configurable.
Hardcoding absolute paths (/home/runner/work/storage/storage/tls/client.crt) might break in local or different CI environments. Consider making them relative or configurable.

README.md Show resolved Hide resolved
Copy link
Member

@gaby gaby left a comment

Choose a reason for hiding this comment

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

LGTM overall, just minor things

.github/workflows/test-valkey.yml Outdated Show resolved Hide resolved
.github/workflows/test-valkey.yml Outdated Show resolved Hide resolved
valkey/README.md Outdated Show resolved Hide resolved
valkey/README.md Outdated Show resolved Hide resolved
valkey/README.md Outdated Show resolved Hide resolved
valkey/go.mod Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (6)
valkey/README.md (6)

12-12: Use a hyphen for improved readability.

“auto pipelining” and “client side caching” can be read more smoothly as “auto-pipelining” and “client-side caching.” Adjusting for best English usage is recommended.

- A fast Valkey Storage that does auto pipelining and supports client side caching.
+ A fast Valkey Storage that does auto-pipelining and supports client-side caching.
🧰 Tools
🪛 LanguageTool

[uncategorized] ~12-~12: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...inter) A fast Valkey Storage that does auto pipelining and supports client side caching. [valk...

(AUTO_HYPHEN)


[uncategorized] ~12-~12: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... that does auto pipelining and supports client side caching. [valkey-io/valkey](https://git...

(EN_COMPOUND_ADJECTIVE_INTERNAL)


14-14: Avoid using emphasis for headings.

Markdownlint suggests using a proper heading syntax instead of bold text for "Note: Requires Go 1.23 and above" to ensure semantic structure.

- **Note: Requires Go 1.23 and above**
+ ### Note
+ Requires Go 1.23 and above
🧰 Tools
🪛 Markdownlint (0.37.0)

14-14: null
Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


49-49: Grammar refinement on “the one of the following.”

The phrase “the one of the following options” is slightly awkward; removing “the” or rephrasing improves readability.

- You can use the one of the following options to create a Valkey Storage:
+ You can use one of the following options to create a Valkey Storage:
🧰 Tools
🪛 LanguageTool

[misspelling] ~49-~49: Make sure that ‘the one of’ is correct and that ‘one’ is a pronoun. Possibly, the ‘the’ is unnecessary or ‘of’ is better expressed with a preposition such as ‘about’ or ‘in’.
Context: ...ofiber/storage/valkey" ``` You can use the one of the following options to create a Valke...

(THE_ONE_OF_DT)


77-84: Replace hard tabs with spaces.

Markdownlint (MD010) flagged lines for containing hard tabs. Replacing them with spaces will ensure formatting consistency across different editors and environments.

-	if err != nil {
-		log.Println(err)
-		return
-	}
-}
+tlsCfg := &tls.Config{
    ...
}
🧰 Tools
🪛 Markdownlint (0.37.0)

77-77: Column: 1
Hard tabs

(MD010, no-hard-tabs)


78-78: Column: 1
Hard tabs

(MD010, no-hard-tabs)


81-81: Column: 1
Hard tabs

(MD010, no-hard-tabs)


82-82: Column: 1
Hard tabs

(MD010, no-hard-tabs)


83-83: Column: 1
Hard tabs

(MD010, no-hard-tabs)


98-188: Replace hard tabs with spaces for all struct fields.

Similar to the previous comment, ensure lines in Config struct definitions also use spaces instead of tabs to avoid indentation issues.

-	type Config struct {
-		Username string
+    type Config struct {
+        Username string
         ...
     }
🧰 Tools
🪛 Markdownlint (0.37.0)

98-98: Column: 1
Hard tabs

(MD010, no-hard-tabs)


99-99: Column: 1
Hard tabs

(MD010, no-hard-tabs)


100-100: Column: 1
Hard tabs

(MD010, no-hard-tabs)


101-101: Column: 1
Hard tabs

(MD010, no-hard-tabs)


103-103: Column: 1
Hard tabs

(MD010, no-hard-tabs)


104-104: Column: 1
Hard tabs

(MD010, no-hard-tabs)


105-105: Column: 1
Hard tabs

(MD010, no-hard-tabs)


106-106: Column: 1
Hard tabs

(MD010, no-hard-tabs)


108-108: Column: 1
Hard tabs

(MD010, no-hard-tabs)


109-109: Column: 1
Hard tabs

(MD010, no-hard-tabs)


110-110: Column: 1
Hard tabs

(MD010, no-hard-tabs)


111-111: Column: 1
Hard tabs

(MD010, no-hard-tabs)


113-113: Column: 1
Hard tabs

(MD010, no-hard-tabs)


114-114: Column: 1
Hard tabs

(MD010, no-hard-tabs)


115-115: Column: 1
Hard tabs

(MD010, no-hard-tabs)


116-116: Column: 1
Hard tabs

(MD010, no-hard-tabs)


117-117: Column: 1
Hard tabs

(MD010, no-hard-tabs)


119-119: Column: 1
Hard tabs

(MD010, no-hard-tabs)


120-120: Column: 1
Hard tabs

(MD010, no-hard-tabs)


121-121: Column: 1
Hard tabs

(MD010, no-hard-tabs)


122-122: Column: 1
Hard tabs

(MD010, no-hard-tabs)


124-124: Column: 1
Hard tabs

(MD010, no-hard-tabs)


125-125: Column: 1
Hard tabs

(MD010, no-hard-tabs)


126-126: Column: 1
Hard tabs

(MD010, no-hard-tabs)


127-127: Column: 1
Hard tabs

(MD010, no-hard-tabs)


129-129: Column: 1
Hard tabs

(MD010, no-hard-tabs)


130-130: Column: 1
Hard tabs

(MD010, no-hard-tabs)


131-131: Column: 1
Hard tabs

(MD010, no-hard-tabs)


132-132: Column: 1
Hard tabs

(MD010, no-hard-tabs)


134-134: Column: 1
Hard tabs

(MD010, no-hard-tabs)


135-135: Column: 1
Hard tabs

(MD010, no-hard-tabs)


136-136: Column: 1
Hard tabs

(MD010, no-hard-tabs)


137-137: Column: 1
Hard tabs

(MD010, no-hard-tabs)


139-139: Column: 1
Hard tabs

(MD010, no-hard-tabs)


140-140: Column: 1
Hard tabs

(MD010, no-hard-tabs)


141-141: Column: 1
Hard tabs

(MD010, no-hard-tabs)


142-142: Column: 1
Hard tabs

(MD010, no-hard-tabs)


144-144: Column: 1
Hard tabs

(MD010, no-hard-tabs)


145-145: Column: 1
Hard tabs

(MD010, no-hard-tabs)


146-146: Column: 1
Hard tabs

(MD010, no-hard-tabs)


147-147: Column: 1
Hard tabs

(MD010, no-hard-tabs)


149-149: Column: 1
Hard tabs

(MD010, no-hard-tabs)


150-150: Column: 1
Hard tabs

(MD010, no-hard-tabs)


151-151: Column: 1
Hard tabs

(MD010, no-hard-tabs)


152-152: Column: 1
Hard tabs

(MD010, no-hard-tabs)


154-154: Column: 1
Hard tabs

(MD010, no-hard-tabs)


155-155: Column: 1
Hard tabs

(MD010, no-hard-tabs)


156-156: Column: 1
Hard tabs

(MD010, no-hard-tabs)


157-157: Column: 1
Hard tabs

(MD010, no-hard-tabs)


159-159: Column: 1
Hard tabs

(MD010, no-hard-tabs)


160-160: Column: 1
Hard tabs

(MD010, no-hard-tabs)


161-161: Column: 1
Hard tabs

(MD010, no-hard-tabs)


162-162: Column: 1
Hard tabs

(MD010, no-hard-tabs)


164-164: Column: 1
Hard tabs

(MD010, no-hard-tabs)


165-165: Column: 1
Hard tabs

(MD010, no-hard-tabs)


166-166: Column: 1
Hard tabs

(MD010, no-hard-tabs)


167-167: Column: 1
Hard tabs

(MD010, no-hard-tabs)


169-169: Column: 1
Hard tabs

(MD010, no-hard-tabs)


170-170: Column: 1
Hard tabs

(MD010, no-hard-tabs)


171-171: Column: 1
Hard tabs

(MD010, no-hard-tabs)


172-172: Column: 1
Hard tabs

(MD010, no-hard-tabs)


174-174: Column: 1
Hard tabs

(MD010, no-hard-tabs)


175-175: Column: 1
Hard tabs

(MD010, no-hard-tabs)


176-176: Column: 1
Hard tabs

(MD010, no-hard-tabs)


177-177: Column: 1
Hard tabs

(MD010, no-hard-tabs)


179-179: Column: 1
Hard tabs

(MD010, no-hard-tabs)


180-180: Column: 1
Hard tabs

(MD010, no-hard-tabs)


181-181: Column: 1
Hard tabs

(MD010, no-hard-tabs)


182-182: Column: 1
Hard tabs

(MD010, no-hard-tabs)


184-184: Column: 1
Hard tabs

(MD010, no-hard-tabs)


185-185: Column: 1
Hard tabs

(MD010, no-hard-tabs)


186-186: Column: 1
Hard tabs

(MD010, no-hard-tabs)


187-187: Column: 1
Hard tabs

(MD010, no-hard-tabs)


194-211: Remove hard tabs in default config initialization.

Maintain consistent spacing to comply with linting best practices and ensure universal readability.

-	var ConfigDefault = Config{
-		Username: "",
+    var ConfigDefault = Config{
+        Username: "",
         ...
     }
🧰 Tools
🪛 Markdownlint (0.37.0)

194-194: Column: 1
Hard tabs

(MD010, no-hard-tabs)


195-195: Column: 1
Hard tabs

(MD010, no-hard-tabs)


196-196: Column: 1
Hard tabs

(MD010, no-hard-tabs)


197-197: Column: 1
Hard tabs

(MD010, no-hard-tabs)


198-198: Column: 1
Hard tabs

(MD010, no-hard-tabs)


199-199: Column: 1
Hard tabs

(MD010, no-hard-tabs)


200-200: Column: 1
Hard tabs

(MD010, no-hard-tabs)


201-201: Column: 1
Hard tabs

(MD010, no-hard-tabs)


202-202: Column: 1
Hard tabs

(MD010, no-hard-tabs)


203-203: Column: 1
Hard tabs

(MD010, no-hard-tabs)


204-204: Column: 1
Hard tabs

(MD010, no-hard-tabs)


205-205: Column: 1
Hard tabs

(MD010, no-hard-tabs)


206-206: Column: 1
Hard tabs

(MD010, no-hard-tabs)


207-207: Column: 1
Hard tabs

(MD010, no-hard-tabs)


208-208: Column: 1
Hard tabs

(MD010, no-hard-tabs)


209-209: Column: 1
Hard tabs

(MD010, no-hard-tabs)


210-210: Column: 1
Hard tabs

(MD010, no-hard-tabs)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9013b62 and 9fa1710.

⛔ Files ignored due to path filters (2)
  • .github/workflows/test-valkey.yml is excluded by !**/*.yml
  • valkey/go.mod is excluded by !**/*.mod
📒 Files selected for processing (1)
  • valkey/README.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
valkey/README.md

[style] ~6-~6: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 1155 characters long)
Context: ...b/v/tag/gofiber/storage?filter=valkey*) Discord Test ![Security](https://img.shields.io/github...

(EN_EXCESSIVE_EXCLAMATION)


[uncategorized] ~12-~12: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...inter) A fast Valkey Storage that does auto pipelining and supports client side caching. [valk...

(AUTO_HYPHEN)


[uncategorized] ~12-~12: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... that does auto pipelining and supports client side caching. [valkey-io/valkey](https://git...

(EN_COMPOUND_ADJECTIVE_INTERNAL)


[misspelling] ~49-~49: Make sure that ‘the one of’ is correct and that ‘one’ is a pronoun. Possibly, the ‘the’ is unnecessary or ‘of’ is better expressed with a preposition such as ‘about’ or ‘in’.
Context: ...ofiber/storage/valkey" ``` You can use the one of the following options to create a Valke...

(THE_ONE_OF_DT)

🪛 Markdownlint (0.37.0)
valkey/README.md

77-77: Column: 1
Hard tabs

(MD010, no-hard-tabs)


78-78: Column: 1
Hard tabs

(MD010, no-hard-tabs)


81-81: Column: 1
Hard tabs

(MD010, no-hard-tabs)


82-82: Column: 1
Hard tabs

(MD010, no-hard-tabs)


83-83: Column: 1
Hard tabs

(MD010, no-hard-tabs)


98-98: Column: 1
Hard tabs

(MD010, no-hard-tabs)


99-99: Column: 1
Hard tabs

(MD010, no-hard-tabs)


100-100: Column: 1
Hard tabs

(MD010, no-hard-tabs)


101-101: Column: 1
Hard tabs

(MD010, no-hard-tabs)


103-103: Column: 1
Hard tabs

(MD010, no-hard-tabs)


104-104: Column: 1
Hard tabs

(MD010, no-hard-tabs)


105-105: Column: 1
Hard tabs

(MD010, no-hard-tabs)


106-106: Column: 1
Hard tabs

(MD010, no-hard-tabs)


108-108: Column: 1
Hard tabs

(MD010, no-hard-tabs)


109-109: Column: 1
Hard tabs

(MD010, no-hard-tabs)


110-110: Column: 1
Hard tabs

(MD010, no-hard-tabs)


111-111: Column: 1
Hard tabs

(MD010, no-hard-tabs)


113-113: Column: 1
Hard tabs

(MD010, no-hard-tabs)


114-114: Column: 1
Hard tabs

(MD010, no-hard-tabs)


115-115: Column: 1
Hard tabs

(MD010, no-hard-tabs)


116-116: Column: 1
Hard tabs

(MD010, no-hard-tabs)


117-117: Column: 1
Hard tabs

(MD010, no-hard-tabs)


119-119: Column: 1
Hard tabs

(MD010, no-hard-tabs)


120-120: Column: 1
Hard tabs

(MD010, no-hard-tabs)


121-121: Column: 1
Hard tabs

(MD010, no-hard-tabs)


122-122: Column: 1
Hard tabs

(MD010, no-hard-tabs)


124-124: Column: 1
Hard tabs

(MD010, no-hard-tabs)


125-125: Column: 1
Hard tabs

(MD010, no-hard-tabs)


126-126: Column: 1
Hard tabs

(MD010, no-hard-tabs)


127-127: Column: 1
Hard tabs

(MD010, no-hard-tabs)


129-129: Column: 1
Hard tabs

(MD010, no-hard-tabs)


130-130: Column: 1
Hard tabs

(MD010, no-hard-tabs)


131-131: Column: 1
Hard tabs

(MD010, no-hard-tabs)


132-132: Column: 1
Hard tabs

(MD010, no-hard-tabs)


134-134: Column: 1
Hard tabs

(MD010, no-hard-tabs)


135-135: Column: 1
Hard tabs

(MD010, no-hard-tabs)


136-136: Column: 1
Hard tabs

(MD010, no-hard-tabs)


137-137: Column: 1
Hard tabs

(MD010, no-hard-tabs)


139-139: Column: 1
Hard tabs

(MD010, no-hard-tabs)


140-140: Column: 1
Hard tabs

(MD010, no-hard-tabs)


141-141: Column: 1
Hard tabs

(MD010, no-hard-tabs)


142-142: Column: 1
Hard tabs

(MD010, no-hard-tabs)


144-144: Column: 1
Hard tabs

(MD010, no-hard-tabs)


145-145: Column: 1
Hard tabs

(MD010, no-hard-tabs)


146-146: Column: 1
Hard tabs

(MD010, no-hard-tabs)


147-147: Column: 1
Hard tabs

(MD010, no-hard-tabs)


149-149: Column: 1
Hard tabs

(MD010, no-hard-tabs)


150-150: Column: 1
Hard tabs

(MD010, no-hard-tabs)


151-151: Column: 1
Hard tabs

(MD010, no-hard-tabs)


152-152: Column: 1
Hard tabs

(MD010, no-hard-tabs)


154-154: Column: 1
Hard tabs

(MD010, no-hard-tabs)


155-155: Column: 1
Hard tabs

(MD010, no-hard-tabs)


156-156: Column: 1
Hard tabs

(MD010, no-hard-tabs)


157-157: Column: 1
Hard tabs

(MD010, no-hard-tabs)


159-159: Column: 1
Hard tabs

(MD010, no-hard-tabs)


160-160: Column: 1
Hard tabs

(MD010, no-hard-tabs)


161-161: Column: 1
Hard tabs

(MD010, no-hard-tabs)


162-162: Column: 1
Hard tabs

(MD010, no-hard-tabs)


164-164: Column: 1
Hard tabs

(MD010, no-hard-tabs)


165-165: Column: 1
Hard tabs

(MD010, no-hard-tabs)


166-166: Column: 1
Hard tabs

(MD010, no-hard-tabs)


167-167: Column: 1
Hard tabs

(MD010, no-hard-tabs)


169-169: Column: 1
Hard tabs

(MD010, no-hard-tabs)


170-170: Column: 1
Hard tabs

(MD010, no-hard-tabs)


171-171: Column: 1
Hard tabs

(MD010, no-hard-tabs)


172-172: Column: 1
Hard tabs

(MD010, no-hard-tabs)


174-174: Column: 1
Hard tabs

(MD010, no-hard-tabs)


175-175: Column: 1
Hard tabs

(MD010, no-hard-tabs)


176-176: Column: 1
Hard tabs

(MD010, no-hard-tabs)


177-177: Column: 1
Hard tabs

(MD010, no-hard-tabs)


179-179: Column: 1
Hard tabs

(MD010, no-hard-tabs)


180-180: Column: 1
Hard tabs

(MD010, no-hard-tabs)


181-181: Column: 1
Hard tabs

(MD010, no-hard-tabs)


182-182: Column: 1
Hard tabs

(MD010, no-hard-tabs)


184-184: Column: 1
Hard tabs

(MD010, no-hard-tabs)


185-185: Column: 1
Hard tabs

(MD010, no-hard-tabs)


186-186: Column: 1
Hard tabs

(MD010, no-hard-tabs)


187-187: Column: 1
Hard tabs

(MD010, no-hard-tabs)


194-194: Column: 1
Hard tabs

(MD010, no-hard-tabs)


195-195: Column: 1
Hard tabs

(MD010, no-hard-tabs)


196-196: Column: 1
Hard tabs

(MD010, no-hard-tabs)


197-197: Column: 1
Hard tabs

(MD010, no-hard-tabs)


198-198: Column: 1
Hard tabs

(MD010, no-hard-tabs)


199-199: Column: 1
Hard tabs

(MD010, no-hard-tabs)


200-200: Column: 1
Hard tabs

(MD010, no-hard-tabs)


201-201: Column: 1
Hard tabs

(MD010, no-hard-tabs)


202-202: Column: 1
Hard tabs

(MD010, no-hard-tabs)


203-203: Column: 1
Hard tabs

(MD010, no-hard-tabs)


204-204: Column: 1
Hard tabs

(MD010, no-hard-tabs)


205-205: Column: 1
Hard tabs

(MD010, no-hard-tabs)


206-206: Column: 1
Hard tabs

(MD010, no-hard-tabs)


207-207: Column: 1
Hard tabs

(MD010, no-hard-tabs)


208-208: Column: 1
Hard tabs

(MD010, no-hard-tabs)


209-209: Column: 1
Hard tabs

(MD010, no-hard-tabs)


210-210: Column: 1
Hard tabs

(MD010, no-hard-tabs)


14-14: null
Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Compare
🔇 Additional comments (1)
valkey/README.md (1)

1-3: Title and metadata look correct.

The top front matter with ID and title is well-structured, and the info is aligned with the new Valkey driver usage. No issues found.

@gaby
Copy link
Member

gaby commented Jan 6, 2025

The redis setup action is broken, lol. Let me report it.

@grivera64
Copy link
Member Author

The redis setup action is broken, lol. Let me report it.

Interesting 😂

It seems that this issue is due to the redis setup action using their own tar files of each redis/valkey version rather than searching on valkey-io/valkey's release page for downloading.

@gaby
Copy link
Member

gaby commented Jan 7, 2025

The redis setup action is broken, lol. Let me report it.

Interesting 😂

It seems that this issue is due to the redis setup action using their own tar files of each redis/valkey version rather than searching on valkey-io/valkey's release page for downloading.

Yes, and the latest release is missing 8.x

I guess we can remove it for now, until they fix that issue.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (3)
valkey/README.md (3)

12-12: Improve readability with proper hyphenation.

The introduction contains compound adjectives that should be hyphenated for clarity:

  • "auto pipelining" → "auto-pipelining"
  • "client side caching" → "client-side caching"
-A fast Valkey Storage that does auto pipelining and supports client side caching. Implementation is based on [valkey-io/valkey](https://github.com/valkey-io/valkey-go).
+A fast Valkey Storage that does auto-pipelining and supports client-side caching. Implementation is based on [valkey-io/valkey](https://github.com/valkey-io/valkey-go).
🧰 Tools
🪛 LanguageTool

[uncategorized] ~12-~12: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...inter) A fast Valkey Storage that does auto pipelining and supports client side caching. Imple...

(AUTO_HYPHEN)


[uncategorized] ~12-~12: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... that does auto pipelining and supports client side caching. Implementation is based on [va...

(EN_COMPOUND_ADJECTIVE_INTERNAL)


56-56: Fix grammatical error in the examples introduction.

The phrase "the one of" is grammatically incorrect.

-You can use the one of the following options to create a Valkey Storage:
+You can use one of the following options to create a Valkey Storage:
🧰 Tools
🪛 LanguageTool

[misspelling] ~56-~56: Make sure that ‘the one of’ is correct and that ‘one’ is a pronoun. Possibly, the ‘the’ is unnecessary or ‘of’ is better expressed with a preposition such as ‘about’ or ‘in’.
Context: ...ofiber/storage/valkey" ``` You can use the one of the following options to create a Valke...

(THE_ONE_OF_DT)


150-150: Fix documentation error in RingScaleEachConn description.

The comment references itself in the default value explanation, which is unclear.

-	// Optional. The default is RingScaleEachConn, which results into having a ring of size 2^10 for each connection.
+	// Optional. The default is 10, which results in having a ring of size 2^10 for each connection.
🧰 Tools
🪛 Markdownlint (0.37.0)

150-150: Column: 1
Hard tabs

(MD010, no-hard-tabs)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9fa1710 and 12ea6b4.

⛔ Files ignored due to path filters (1)
  • .github/workflows/test-valkey.yml is excluded by !**/*.yml
📒 Files selected for processing (1)
  • valkey/README.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
valkey/README.md

[style] ~6-~6: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 1178 characters long)
Context: ...b/v/tag/gofiber/storage?filter=valkey*) Discord Test ![Security](https://img.shields.io/github...

(EN_EXCESSIVE_EXCLAMATION)


[uncategorized] ~12-~12: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...inter) A fast Valkey Storage that does auto pipelining and supports client side caching. Imple...

(AUTO_HYPHEN)


[uncategorized] ~12-~12: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... that does auto pipelining and supports client side caching. Implementation is based on [va...

(EN_COMPOUND_ADJECTIVE_INTERNAL)


[misspelling] ~56-~56: Make sure that ‘the one of’ is correct and that ‘one’ is a pronoun. Possibly, the ‘the’ is unnecessary or ‘of’ is better expressed with a preposition such as ‘about’ or ‘in’.
Context: ...ofiber/storage/valkey" ``` You can use the one of the following options to create a Valke...

(THE_ONE_OF_DT)

🪛 Markdownlint (0.37.0)
valkey/README.md

85-85: Column: 1
Hard tabs

(MD010, no-hard-tabs)


86-86: Column: 1
Hard tabs

(MD010, no-hard-tabs)


89-89: Column: 1
Hard tabs

(MD010, no-hard-tabs)


90-90: Column: 1
Hard tabs

(MD010, no-hard-tabs)


91-91: Column: 1
Hard tabs

(MD010, no-hard-tabs)


107-107: Column: 1
Hard tabs

(MD010, no-hard-tabs)


108-108: Column: 1
Hard tabs

(MD010, no-hard-tabs)


109-109: Column: 1
Hard tabs

(MD010, no-hard-tabs)


110-110: Column: 1
Hard tabs

(MD010, no-hard-tabs)


112-112: Column: 1
Hard tabs

(MD010, no-hard-tabs)


113-113: Column: 1
Hard tabs

(MD010, no-hard-tabs)


114-114: Column: 1
Hard tabs

(MD010, no-hard-tabs)


115-115: Column: 1
Hard tabs

(MD010, no-hard-tabs)


117-117: Column: 1
Hard tabs

(MD010, no-hard-tabs)


118-118: Column: 1
Hard tabs

(MD010, no-hard-tabs)


119-119: Column: 1
Hard tabs

(MD010, no-hard-tabs)


120-120: Column: 1
Hard tabs

(MD010, no-hard-tabs)


122-122: Column: 1
Hard tabs

(MD010, no-hard-tabs)


123-123: Column: 1
Hard tabs

(MD010, no-hard-tabs)


124-124: Column: 1
Hard tabs

(MD010, no-hard-tabs)


125-125: Column: 1
Hard tabs

(MD010, no-hard-tabs)


126-126: Column: 1
Hard tabs

(MD010, no-hard-tabs)


128-128: Column: 1
Hard tabs

(MD010, no-hard-tabs)


129-129: Column: 1
Hard tabs

(MD010, no-hard-tabs)


130-130: Column: 1
Hard tabs

(MD010, no-hard-tabs)


131-131: Column: 1
Hard tabs

(MD010, no-hard-tabs)


133-133: Column: 1
Hard tabs

(MD010, no-hard-tabs)


134-134: Column: 1
Hard tabs

(MD010, no-hard-tabs)


135-135: Column: 1
Hard tabs

(MD010, no-hard-tabs)


136-136: Column: 1
Hard tabs

(MD010, no-hard-tabs)


138-138: Column: 1
Hard tabs

(MD010, no-hard-tabs)


139-139: Column: 1
Hard tabs

(MD010, no-hard-tabs)


140-140: Column: 1
Hard tabs

(MD010, no-hard-tabs)


141-141: Column: 1
Hard tabs

(MD010, no-hard-tabs)


143-143: Column: 1
Hard tabs

(MD010, no-hard-tabs)


144-144: Column: 1
Hard tabs

(MD010, no-hard-tabs)


145-145: Column: 1
Hard tabs

(MD010, no-hard-tabs)


146-146: Column: 1
Hard tabs

(MD010, no-hard-tabs)


148-148: Column: 1
Hard tabs

(MD010, no-hard-tabs)


149-149: Column: 1
Hard tabs

(MD010, no-hard-tabs)


150-150: Column: 1
Hard tabs

(MD010, no-hard-tabs)


151-151: Column: 1
Hard tabs

(MD010, no-hard-tabs)


153-153: Column: 1
Hard tabs

(MD010, no-hard-tabs)


154-154: Column: 1
Hard tabs

(MD010, no-hard-tabs)


155-155: Column: 1
Hard tabs

(MD010, no-hard-tabs)


156-156: Column: 1
Hard tabs

(MD010, no-hard-tabs)


158-158: Column: 1
Hard tabs

(MD010, no-hard-tabs)


159-159: Column: 1
Hard tabs

(MD010, no-hard-tabs)


160-160: Column: 1
Hard tabs

(MD010, no-hard-tabs)


161-161: Column: 1
Hard tabs

(MD010, no-hard-tabs)


163-163: Column: 1
Hard tabs

(MD010, no-hard-tabs)


164-164: Column: 1
Hard tabs

(MD010, no-hard-tabs)


165-165: Column: 1
Hard tabs

(MD010, no-hard-tabs)


166-166: Column: 1
Hard tabs

(MD010, no-hard-tabs)


168-168: Column: 1
Hard tabs

(MD010, no-hard-tabs)


169-169: Column: 1
Hard tabs

(MD010, no-hard-tabs)


170-170: Column: 1
Hard tabs

(MD010, no-hard-tabs)


171-171: Column: 1
Hard tabs

(MD010, no-hard-tabs)


173-173: Column: 1
Hard tabs

(MD010, no-hard-tabs)


174-174: Column: 1
Hard tabs

(MD010, no-hard-tabs)


175-175: Column: 1
Hard tabs

(MD010, no-hard-tabs)


176-176: Column: 1
Hard tabs

(MD010, no-hard-tabs)


178-178: Column: 1
Hard tabs

(MD010, no-hard-tabs)


179-179: Column: 1
Hard tabs

(MD010, no-hard-tabs)


180-180: Column: 1
Hard tabs

(MD010, no-hard-tabs)


181-181: Column: 1
Hard tabs

(MD010, no-hard-tabs)


183-183: Column: 1
Hard tabs

(MD010, no-hard-tabs)


184-184: Column: 1
Hard tabs

(MD010, no-hard-tabs)


185-185: Column: 1
Hard tabs

(MD010, no-hard-tabs)


186-186: Column: 1
Hard tabs

(MD010, no-hard-tabs)


188-188: Column: 1
Hard tabs

(MD010, no-hard-tabs)


189-189: Column: 1
Hard tabs

(MD010, no-hard-tabs)


190-190: Column: 1
Hard tabs

(MD010, no-hard-tabs)


191-191: Column: 1
Hard tabs

(MD010, no-hard-tabs)


193-193: Column: 1
Hard tabs

(MD010, no-hard-tabs)


194-194: Column: 1
Hard tabs

(MD010, no-hard-tabs)


195-195: Column: 1
Hard tabs

(MD010, no-hard-tabs)


196-196: Column: 1
Hard tabs

(MD010, no-hard-tabs)


204-204: Column: 1
Hard tabs

(MD010, no-hard-tabs)


205-205: Column: 1
Hard tabs

(MD010, no-hard-tabs)


206-206: Column: 1
Hard tabs

(MD010, no-hard-tabs)


207-207: Column: 1
Hard tabs

(MD010, no-hard-tabs)


208-208: Column: 1
Hard tabs

(MD010, no-hard-tabs)


209-209: Column: 1
Hard tabs

(MD010, no-hard-tabs)


210-210: Column: 1
Hard tabs

(MD010, no-hard-tabs)


211-211: Column: 1
Hard tabs

(MD010, no-hard-tabs)


212-212: Column: 1
Hard tabs

(MD010, no-hard-tabs)


213-213: Column: 1
Hard tabs

(MD010, no-hard-tabs)


214-214: Column: 1
Hard tabs

(MD010, no-hard-tabs)


215-215: Column: 1
Hard tabs

(MD010, no-hard-tabs)


216-216: Column: 1
Hard tabs

(MD010, no-hard-tabs)


217-217: Column: 1
Hard tabs

(MD010, no-hard-tabs)


218-218: Column: 1
Hard tabs

(MD010, no-hard-tabs)


219-219: Column: 1
Hard tabs

(MD010, no-hard-tabs)


220-220: Column: 1
Hard tabs

(MD010, no-hard-tabs)

⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: Analyse
  • GitHub Check: Compare
🔇 Additional comments (2)
valkey/README.md (2)

36-36: Update Go version requirement.

As discussed in the PR comments, specify Go version 1.23 as the minimum requirement.

-The valkey driver is tested on the latest two [Go version](https://golang.org/dl/) with support for modules. So make sure to initialize one first if you didn't do that yet:
+The valkey driver requires Go 1.23 or later with support for modules. Make sure to initialize one first if you haven't done so:
✅ Verification successful

The Go version update is accurate and matches the codebase requirements

The suggested change correctly reflects the Go 1.23 requirement specified in the valkey module's go.mod file. The update improves documentation clarity by replacing the vague "latest two versions" with the specific minimum version requirement.

🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found


Script:

#!/bin/bash
# Find and show go.mod file
fd -t f '^go\.mod$' -x cat {}

# Search for Go version mentions in documentation
fd -e md -e txt -X rg -i "go (version|[0-9]+\.[0-9]+)"

# Search for Go version related build constraints
rg -i "^//\s*\+build\s+go[0-9]" 

# Search for any other Go version requirements in code
rg -i "requires?\s+go\s*[0-9]"

Length of output: 31996


203-221: Verify configuration defaults.

The default configuration looks good, but we should verify that these values match the ones in the Valkey implementation.

✅ Verification successful

Default configuration values are correctly documented

The default values in the README.md match the implementation:

  • DefaultCacheBytes: 128 * (1 << 20)
  • DefaultReadBuffer: 1 << 19 (0.5 MiB)
  • DefaultWriteBuffer: 1 << 19 (0.5 MiB)
  • DefaultPoolSize: 1000
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the default configuration values match the Valkey implementation

# Search for default constant declarations in the Valkey package
rg -U "(?s)const\s*\(\s*.*Default.*\)" 

# Search for any other default values that might be defined separately
rg "Default[A-Za-z]+\s*=\s*" 

Length of output: 8440


Script:

#!/bin/bash
# Search for the specific default constants we need to verify
ast-grep --pattern 'const DefaultCacheBytes = $_'
ast-grep --pattern 'const DefaultRingScale = $_'
ast-grep --pattern 'const DefaultReadBuffer = $_'
ast-grep --pattern 'const DefaultWriteBuffer = $_'
ast-grep --pattern 'const DefaultPoolSize = $_'

# Also search with ripgrep as a backup
rg "const\s+(Default(CacheBytes|RingScale|ReadBuffer|WriteBuffer|PoolSize))\s*=\s*" -A 1

Length of output: 348


Script:

#!/bin/bash
# First, find the valkey package location
fd -t f -e go . | xargs head -n 1 | grep -l "package valkey"

# Search for default values in const blocks and var declarations
rg -U "(?s)const\s*\(\s*[^)]*Default[^)]*\)" 
rg "var\s+Default(CacheBytes|RingScale|ReadBuffer|WriteBuffer|PoolSize)\s*=\s*" 

# Search for any line containing these default values
rg "Default(CacheBytes|RingScale|ReadBuffer|WriteBuffer|PoolSize)"

Length of output: 3913

🧰 Tools
🪛 Markdownlint (0.37.0)

204-204: Column: 1
Hard tabs

(MD010, no-hard-tabs)


205-205: Column: 1
Hard tabs

(MD010, no-hard-tabs)


206-206: Column: 1
Hard tabs

(MD010, no-hard-tabs)


207-207: Column: 1
Hard tabs

(MD010, no-hard-tabs)


208-208: Column: 1
Hard tabs

(MD010, no-hard-tabs)


209-209: Column: 1
Hard tabs

(MD010, no-hard-tabs)


210-210: Column: 1
Hard tabs

(MD010, no-hard-tabs)


211-211: Column: 1
Hard tabs

(MD010, no-hard-tabs)


212-212: Column: 1
Hard tabs

(MD010, no-hard-tabs)


213-213: Column: 1
Hard tabs

(MD010, no-hard-tabs)


214-214: Column: 1
Hard tabs

(MD010, no-hard-tabs)


215-215: Column: 1
Hard tabs

(MD010, no-hard-tabs)


216-216: Column: 1
Hard tabs

(MD010, no-hard-tabs)


217-217: Column: 1
Hard tabs

(MD010, no-hard-tabs)


218-218: Column: 1
Hard tabs

(MD010, no-hard-tabs)


219-219: Column: 1
Hard tabs

(MD010, no-hard-tabs)


220-220: Column: 1
Hard tabs

(MD010, no-hard-tabs)

valkey/README.md Show resolved Hide resolved
Copy link
Member

@gaby gaby left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@ReneWerner87 ReneWerner87 merged commit 87c2b45 into gofiber:main Jan 7, 2025
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🚀 [Feature]: Implement Valkey Storage
3 participants