storage icon indicating copy to clipboard operation
storage copied to clipboard

🔥 feat: Support for DisableStartupCheck for several drivers

Open gaby opened this issue 3 months ago • 2 comments

Summary

  • Add support for DisableStartupCheck to several storage drivers.

We can add this feature to other drivers in the future if needed.

Fixes #1924

Summary by CodeRabbit

  • New Features
    • Added DisableStartupCheck across Memcache, Postgres, Redis and Rueidis to optionally skip startup connectivity checks and resets (default: false).
  • Bug Fixes
    • Rueidis: avoid panics on uninitialized clients by deferring initialization errors and making Close/operations safe when startup checks are disabled.
  • Documentation
    • Updated config docs to include DisableStartupCheck and its default.
  • Tests
    • Added tests verifying stores construct and close without startup checks enabled.

gaby avatar Oct 13 '25 17:10 gaby

Summary of Changes

Hello @gaby, I'm Gemini Code Assist[^1]! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the flexibility of several database and caching drivers (Memcache, MySQL, PostgreSQL, and Redis) by introducing a DisableStartupCheck configuration option. This allows developers to control whether an immediate connection validation is performed upon driver initialization, providing more robust startup behavior for applications that might encounter transient database unavailability or prefer to handle connection issues asynchronously.

Highlights

  • New DisableStartupCheck Option: Introduced a new DisableStartupCheck boolean option in the configuration for Memcache, MySQL, PostgreSQL, and Redis drivers. When set to true, this option prevents the driver from performing initial connection validation (like pinging the database or checking schema) during its New function call.
  • Deferred Connection Validation: The primary impact of DisableStartupCheck is to allow applications to initialize drivers without an immediate, blocking connection check. This can be useful in scenarios where the database might not be immediately available at application startup, deferring connection validation to the first actual operation.
  • Updated Documentation and Tests: The README.md files for each affected driver have been updated to document the new configuration option. Additionally, new test cases have been added to verify the functionality of DisableStartupCheck, ensuring that the New function does not panic when startup checks are disabled and a connection cannot be established.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with :thumbsup: and :thumbsdown: on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

[^1]: Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

gemini-code-assist[bot] avatar Oct 13 '25 17:10 gemini-code-assist[bot]

[!NOTE]

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds a DisableStartupCheck boolean across memcache, postgres, redis, and rueidis configs/defaults; constructors now optionally skip startup ping/reset when set. Rueidis additionally records initialization errors (initErr), defers error propagation to operations, and guards Close/get/set/delete/reset against nil client. READMEs and tests updated.

Changes

Cohort / File(s) Summary
Memcache config & docs
memcache/config.go, memcache/README.md
Add DisableStartupCheck bool to Config and include DisableStartupCheck: false in ConfigDefault; README updated.
Memcache constructor & tests
memcache/memcache.go, memcache/memcache_test.go
Guard startup Ping and optional DeleteAll behind !cfg.DisableStartupCheck; add test Test_Memcache_New_DisableStartupCheck.
Postgres config & docs
postgres/config.go, postgres/README.md
Add DisableStartupCheck bool to Config and ConfigDefault; README updated.
Postgres constructor & tests
postgres/postgres.go, postgres/postgres_test.go
Gate startup ping, schema/table checks, optional drop/init, and checkSchema behind !cfg.DisableStartupCheck; add test for disabled startup check.
Redis config & docs
redis/config.go, redis/README.md
Add DisableStartupCheck bool to Config and ConfigDefault; README updated.
Redis constructor & tests
redis/redis.go, redis/redis_test.go
Wrap startup Ping and optional FlushDB with !cfg.DisableStartupCheck; add Test_Redis_New_DisableStartupCheck.
Rueidis config & docs
rueidis/config.go, rueidis/README.md
Add DisableStartupCheck bool to Config, include in ConfigDefault, and ensure configDefault propagates user flag.
Rueidis implementation & tests
rueidis/rueidis.go, rueidis/rueidis_test.go
Add initErr error to Storage; when DisableStartupCheck is true, constructor may store init error and return a Storage with nil client; Get/Set/Delete/Reset short-circuit with initErr if uninitialized; Close guarded for nil client; add test for disabled startup check.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor App
  participant C as Config
  participant New as New(...)
  participant DB as Backend

  App->>C: Provide Config (DisableStartupCheck = true/false)
  App->>New: Call New(C)
  New->>DB: Establish connection
  alt DisableStartupCheck = false
    New->>DB: Ping (startup validation)
    alt Reset enabled
      New->>DB: Reset (Flush/Drop/DeleteAll)
      DB-->>New: Reset result
    end
    DB-->>New: Ping result
    New-->>App: Return initialized Storage (db != nil, initErr nil)
  else DisableStartupCheck = true
    Note over New,DB: Skip Ping and Reset
    alt connection error occurred
      New-->>New: store initErr (db may be nil)
      New-->>App: Return Storage with initErr
    else
      New-->>App: Return Storage (db set, initErr nil)
    end
  end
  rect rgba(0,128,0,0.08)
    Note right of App: Subsequent ops (Get/Set/Delete/Reset) check initErr and nil db
  end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • ReneWerner87
  • efectn
  • sixcolors

Poem

I twitch my whiskers, skip the ping,
a quiet hop instead of fling.
If startup sleeps or stumbles slow,
I tuck the error where it’ll grow.
We’ll fetch and set when carrots sing. 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Linked Issues Check ⚠️ Warning While the pull request adds a DisableStartupCheck flag to Redis to defer startup I/O, it does not change the Redis constructor signature to return (*Storage, error) or remove panic calls when the flag is unset, which were explicit requirements in issue #1924. Update the Redis storage constructors to return (*Storage, error) and replace existing panic calls with error returns so that connectivity failures can be handled gracefully in accordance with issue #1924.
Out of Scope Changes Check ⚠️ Warning The pull request extends DisableStartupCheck support to Memcache, Postgres, and Rueidis drivers, which are not addressed in the linked Redis issue #1924 and therefore represent out-of-scope changes relative to that bug’s requirements. Either restrict this pull request to Redis-related modifications or split out the additional driver updates into separate pull requests linked to their own issues to maintain focused scope.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly and concisely describes the main enhancement of introducing a DisableStartupCheck option across multiple storage drivers, matching the core change in the pull request without extraneous detail.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment
  • [ ] Commit unit tests in branch add-config-option-to-disable-initial-check

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Oct 13 '25 17:10 coderabbitai[bot]