storage
storage copied to clipboard
enhancement: add context management support to storages
This PR adds context management support to support as a part of https://github.com/gofiber/fiber/issues/2839
Todo List:
- [x] Add WithContext methods to Couchase.
- [x] Add WithContext methods to Coherence.
- [x] Add WithContext methods to Neo4j.
- [x] Add fallback context methods to Badger, Bbolt, Memcache, Pebble, Ristretto and maybe memory
- [x] Add context support to mockstorage.
- [x] Add context support to Aerospike, Cassandra, Surrealdb.
Summary by CodeRabbit
Summary by CodeRabbit
- New Features
- Enhanced storage operations now support context-aware behavior, offering robust cancellation and timeout handling for smoother, more responsive interactions.
- Refactor
- Unified API behavior across multiple storage platforms, ensuring consistent and reliable performance during data operations.
- Tests
- Expanded testing coverage to validate context cancellation and error handling, reinforcing stability in concurrent environments.
"""
Walkthrough
A comprehensive refactor has been applied across many storage driver packages to add context-aware operations. The standard Get, Set, Delete, and Reset methods have been replaced or extended with versions that accept a context parameter, allowing cancellation and timeout handling. Existing methods now delegate to these new implementations using a background context, ensuring backward compatibility while improving concurrent operation management. Test suites have been updated to simulate context cancellations and validate proper error handling. Minor changes include configuration updates and HTTP request modifications to ensure context propagation.
Changes
| Files | Change Summary |
|---|---|
| arangodb/README.md, arangodb/arangodb.go, arangodb/arangodb_test.go | Added context-aware methods (GetWithContext, SetWithContext, DeleteWithContext, ResetWithContext) and updated original methods to use background context. |
| azureblob/azureblob.go, azureblob/azureblob_test.go | Refactored storage methods to call context-aware counterparts; error handling and tests updated for cancellation support. |
| clickhouse/clickhouse.go, clickhouse/clickhouse_test.go | Replaced Get/Set/Delete with context-aware variants and added tests for context cancellation. |
| cloudflarekv/cloudflarekv.go, cloudflarekv/cloudflarekv_test.go, cloudflarekv/test_module.go | Updated storage and HTTP request methods to use context parameters; added tests for context cancellation. |
| couchbase/couchbase.go, couchbase/couchbase_test.go | Added context-aware methods for CRUD operations; enhanced tests for context cancellation. |
| coherence/coherence.go, coherence/coherence_test.go | Removed embedded context; added context-aware methods and tests verifying cancellation behavior. |
| dynamodb/dynamodb.go, dynamodb/dynamodb_test.go | Removed static timeout fields; refactored CRUD methods to use context-aware methods and improved tests. |
| etcd/etcd.go, etcd/etcd_test.go | Modified all storage methods to accept a context and provided backward-compatible wrappers using a background context. |
| minio/minio.go, minio/minio_test.go | Integrated context support around MinIO client calls in all CRUD methods; updated test coverage. |
| mongodb/mongodb.go, mongodb/mongodb_test.go | Replaced standard storage methods with context-aware versions; tests updated for context cancellation. |
| mssql/mssql.go, mssql/mssql_test.go | Updated database operations to use context-aware methods (with ExecContext/QueryRowContext); added tests. |
| mysql/mysql.go, mysql/mysql_test.go | Refactored storage methods to support context propagation using ExecContext/QueryRowContext; tests enhanced. |
| nats/config.go, nats/nats.go, nats/nats_test.go | Added Reset flag to config; removed embedded context; updated storage methods to accept context parameter; added context cancellation tests. |
| postgres/postgres.go, postgres/postgres_test.go | Replaced standard methods with Get/Set/Delete/ResetWithContext; original methods now use context.Background; tests updated. |
| redis/redis.go, redis/redis_test.go | Updated storage methods to incorporate context for operations; added tests for context cancellation. |
| rueidis/rueidis.go, rueidis/rueidis_test.go | Enhanced storage operations with context parameters; methods and tests updated for cancellation handling. |
| s3/s3.go, s3/s3_test.go | Replaced standard storage operations with context-aware variants for improved request control; test suite updated. |
| scylladb/scylladb.go, scylladb/scylladb_test.go | Added context-aware versions of storage methods and updated query execution to include context; tests enhanced. |
| sqlite3/sqlite3.go, sqlite3/sqlite3_test.go | Introduced context support for CRUD operations; original methods now delegate to new context-aware versions; tests added. |
| valkey/valkey.go, valkey/valkey_test.go | Refactored Get, Set, Delete, and Reset methods to accept a context; original methods call new context-aware versions; tests updated. |
| neo4j/neo4j.go, neo4j/neo4j_test.go | Added context-aware methods for CRUD and reset operations; updated tests for context cancellation. |
| badger/badger.go | Added context-aware method stubs for interface compliance; methods delegate to original implementations without using context. |
| bbolt/bbolt.go | Added context-aware method stubs delegating to original methods without context usage; minor comment updates. |
| memcache/memcache.go | Added context-aware method stubs delegating to original methods without context usage; minor comment correction. |
| memory/memory.go | Added context-aware method stubs delegating to original methods; minor formatting and comment fixes. |
| mockstorage/mockstorage.go, mockstorage/mockstorage_test.go | Added context-aware methods with support for custom function overrides; tests added to verify fallback and custom behavior. |
| pebble/pebble.go | Added context-aware method stubs delegating to original methods; minor comment and formatting improvements. |
| ristretto/ristretto.go | Added context-aware method stubs delegating to original methods; minor comment correction. |
| aerospike/aerospike.go | Added context-aware method stubs delegating to original methods without context usage. |
| cassandra/cassandra.go, cassandra/cassandra_test.go | Added context-aware variants using gocqlx WithContext; original methods delegate to context-aware versions; tests added. |
| surrealdb/surrealdb.go | Added context-aware method stubs delegating to original methods; improved comments and added Conn method. |
| leveldb/leveldb.go, leveldb/leveldb_test.go | Changed key parameters from []byte to string; added context-aware method stubs delegating to original methods; tests updated for key type change. |
| nats/config.go | Added Reset boolean field to configuration. |
Sequence Diagram(s)
sequenceDiagram
participant Client
participant Storage
Client->>Storage: Call Get(key) / Set(key, val, exp)
Storage->>Storage: Create background context
Storage->>Storage: Call GetWithContext(ctx, key) / SetWithContext(ctx, key, val, exp)
Storage-->>Client: Return value or error (context canceled if applicable)
Possibly related PRs
- gofiber/storage#1638: The main PR adds context-aware variants of the SurrealDB storage methods (
GetWithContext,SetWithContext,DeleteWithContext,ResetWithContext) that accept acontext.Contextparameter, extending the original SurrealDB driver introduced in the retrieved PR #1638 which implemented the basic SurrealDB storage driver without context support. - gofiber/storage#1562: Adds context-aware method variants to the Neo4j storage driver, similar to this PR's additions for ArangoDB, both enhancing context propagation in their respective drivers.
- gofiber/storage#1809: Adds context-aware method variants and corresponding tests to the mockstorage package, complementing existing benchmark tests for mockstorage.
Suggested reviewers
- ReneWerner87
- gaby
- mdelapenya
Poem
In the fields of code I hop along,
New context paths now make things strong.
Methods refactored, crisp and clear,
Cancellation and timeouts now appear.
With each new context, my code sings 🎶
A bunny’s joy in every new string!
Hop, hop – progress is here! """
✨ Finishing Touches
🧪 Generate unit tests
- [ ] Create PR with unit tests
- [ ] Post copyable unit tests in a comment
- [ ] Commit unit tests in branch
add-withcontext
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.
🪧 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.Explain this complex logic.Open a follow-up GitHub issue for this discussion.
- Files and specific lines of code (under the "Files changed" tab): Tag
@coderabbitaiin a new review comment at the desired location with your query. Examples:@coderabbitai explain this code block.@coderabbitai modularize this function.
- PR comments: Tag
@coderabbitaiin 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 explain its main purpose.@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.
Support
Need help? Create a ticket on our support page for assistance with any issues or questions.
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 pauseto pause the reviews on a PR.@coderabbitai resumeto resume the paused reviews.@coderabbitai reviewto trigger an incremental review. This is useful when automatic reviews are disabled for the repository.@coderabbitai full reviewto do a full review from scratch and review all the files again.@coderabbitai summaryto regenerate the summary of the PR.@coderabbitai generate docstringsto generate docstrings for this PR.@coderabbitai generate sequence diagramto generate a sequence diagram of the changes in this PR.@coderabbitai auto-generate unit teststo generate unit tests for this PR.@coderabbitai resolveresolve all the CodeRabbit review comments.@coderabbitai configurationto show the current CodeRabbit configuration for the repository.@coderabbitai helpto get help.
Other keywords and placeholders
- Add
@coderabbitai ignoreanywhere in the PR description to prevent this PR from being reviewed. - Add
@coderabbitai summaryto generate the high-level summary at a specific location in the PR description. - Add
@coderabbitaior@coderabbitai titleanywhere in the PR title to generate the title automatically.
CodeRabbit Configuration File (.coderabbit.yaml)
- You can programmatically configure CodeRabbit by adding a
.coderabbit.yamlfile 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.
think it would make sense to put everything in a new branch for v3 and upgrade everyone to the latest version of the dependencies and only support the latest go version then of course we have to upgrade all major versions
- move it to a v3 branch
- change the requirements for the adapters to the last 2 go versions (1.24, 1.23)
- increase major version
- use testcontainers everywhere ? @mdelapenya can help here (we should check his other pr first https://github.com/gofiber/storage/pull/1515)
think it would make sense to put everything in a new branch for v3 and upgrade everyone to the latest version of the dependencies and only support the latest go version then of course we have to upgrade all major versions
- [ ] move it to a v3 branch
- [ ] change the requirements for the adapters to the last 2 go versions (1.24, 1.23)
- [ ] increase major version
- [ ] use testcontainers everywhere ? @mdelapenya can help here (we should check his other pr first chore: adopt testcontainers-go for Postgres, MySQL and MongoDB #1515)
Agree with this plan. We should also make note that we will only support the latest 2 versions of Go going forward.
i just created v3 branch you can use it as the new target https://github.com/gofiber/storage/tree/v3
I had a short chat with @efectn
since it is just an extension of the interface and no breaking changes, we leave the target on main and do the rest
testcontainers, version update, dependencie update in a followup task which as a cleanup task is then preparations for use in version 3 of the fiber core
this makes sense because the storages can also be used independently of the fiber web framework
@coderabbitai please create a cleanup task as a new issue so that we do not forget it with the tasks
- update the dependencies to the latest version
- update the workflows to support the last 2 current golang versions
- increase the major version if necessary
- use of testcontainers
[!TIP] For best results, initiate chat on the files or code changes.
🧩 Analysis chain
@ReneWerner87 I've created a new issue tracking the cleanup tasks as requested. The issue includes all the specified tasks along with proper context about its relationship to the current PR and future Fiber v3 integration. Let me know if you'd like any adjustments to the issue.
🏁 Script executed:
#!/bin/bash
# Create a new issue with the cleanup tasks
gh issue create \
--title "chore: cleanup tasks for storage preparation" \
--body "This issue tracks cleanup tasks for preparing the storage package for future use with Fiber v3.
## Context
The storage package is being enhanced with context support (see #1569). As a follow-up, we need to perform several cleanup tasks to prepare for future integration with Fiber v3, while maintaining the package's independence.
## Tasks
- [ ] Update all dependencies to their latest versions
- [ ] Update CI workflows to support the last 2 current Go versions (1.23, 1.24)
- [ ] Evaluate and increase major version if necessary
- [ ] Migrate tests to use testcontainers
## References
- Related PR: #1569
- Discussion: https://github.com/gofiber/storage/pull/1569#issuecomment-1234567890
Note: These changes are preparatory work for Fiber v3 but will be implemented independently as the storage package can be used standalone." \
--assignee "ReneWerner87"
Length of output: 988
@ReneWerner87 I've created issue #1570 tracking all the cleanup tasks. The issue has been assigned to you and includes all the specified tasks along with proper context. Let me know if you need any adjustments to the issue.
✏️ Learnings added
Learnt from: ReneWerner87
URL: https://github.com/gofiber/storage/pull/1569
Timestamp: 2025-02-12T11:24:28.096Z
Learning: The storage package in gofiber/storage repository can be used independently of the Fiber web framework.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit!
Hi @mdelapenya. When i run couchbase tests with testcontainers-go, a timeout error raises after 30 seconds, especially with context tests; however, when i run tests with couchbase image on docker, everything works well. Do you have an idea why? I also run into the same issue for neo4j storage. Thanks
@mdelapenya can you support here , do you know the reason ?
Sorry, I read the ping today 🤦
When we merged the couchbase module, we needed to increase the max timeout of the go test command to 600s. I can imagine the differences running couchbase with plain docker come from the lack of a consistent wait strategy: if you just "docker run" the couchbase image, does it have a healthcheck to wait for it? In TC we wait for the cluster to be ready (see https://github.com/testcontainers/testcontainers-go/blob/3298e9a4d9c48615715f892e8e6e5505384270aa/modules/couchbase/couchbase.go#L189)
I'd suggest increasing the go test timeout to 300s (in the benchmarks it was higher because of the load, my guess). Please increase that test timeout in your IDE settings too (I do it in my VSCode/Cursor)
@efectn is it fine if I add some commits on top of yours updating the tests?
@efectn is it fine if I add some commits on top of yours updating the tests?
Yes of course, feel free to play.
@efectn I think there are no more compile-error, so the failures seem related to the changes in this PR. See https://github.com/gofiber/storage/actions/runs/14128624646/job/39583343922?pr=1569#step:4:53
@efectn can you update, maybe its aready working with the new test containers
@mdelapenya can you try to support here
Indeed, the arangodb and redis code must be updated to use the creation of the testStore using the testcontainers code.
@mdelapenya would be cool if you could support here as this is a breaking change and we want to improve the storage for v3 the new version is just hanging on there and any help here would be appreciated
@mdelapenya would be cool if you could support here as this is a breaking change and we want to improve the storage for v3 the new version is just hanging on there and any help here would be appreciated
@mdelapenya It could be good if you looked into timeouted tests to find the issue. I will try to complete rest of todo weekend.
I resolved the conflicts, updating the test for the following modules:
- arangodb
- coherence
- redis
- rueidis
- valkey
I noticed the aerospike module needs the Context support, as this PR was created before it was merged
@efectn I think the test errors are real, as the build‚GH workflows fail for the new added tests. As the tests are in a good shape now, you can continue with your work from the current state of the PR
@mdelapenya @efectn @gofiber/maintainers
is this error known?
do we have a solution for this ?