fix: redix should be loaded - #71
Conversation
WalkthroughThe Redis session store module is now conditionally defined using Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes
Potential focus areas:
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
lib/anubis/server/session/store/redis.ex(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
lib/anubis/server/session/store/redis.ex (1)
lib/anubis/server/session/supervisor.ex (2)
start_link(25-39)init(104-106)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test (1.18, 26)
- GitHub Check: test (1.18, 28)
🔇 Additional comments (2)
lib/anubis/server/session/store/redis.ex (2)
2-428: ✅ Module implementation looks solidThe Redis store logic itself is well-structured with proper:
- GenServer patterns
- Connection pooling
- Error handling & logging
- Behaviour implementation
The indentation changes are purely mechanical due to the wrapper. No behavioral issues detected in the core logic.
1-1: This review comment is incorrect. The code already implements graceful error handling for missing Redix.The
get_session_store_adapter()function inlib/anubis.exincludes a runtime checkCode.ensure_loaded?(adapter)that returnsnilwhen the Redis module is not available. Therestore_sessions()function in the supervisor explicitly handles thisnilcase and skips session restoration gracefully. This is the intended validation layer—not instart_link/1.The current approach (compile-time conditional module definition + runtime adapter check) is sound for optional dependencies. If Redix is not included and the adapter is configured, the system will simply skip session restoration rather than crash.
A potential minor improvement would be to log a warning when an adapter is explicitly configured but not available, but the current silent no-op is reasonable for truly optional functionality.
Likely an incorrect or invalid review comment.
| @moduledoc """ | ||
| Redis-based session store implementation. | ||
|
|
||
| Uses Redix for Redis communication and provides persistent session storage | ||
| with automatic expiration and connection pooling. | ||
|
|
||
| ## Configuration | ||
|
|
||
| config :anubis_mcp, :session_store, | ||
| adapter: Anubis.Server.Session.Store.Redis, | ||
| redis_url: "redis://localhost:6379/0", | ||
| pool_size: 10, | ||
| ttl: 1_800_000, # 30 minutes in milliseconds | ||
| namespace: "anubis:sessions", | ||
| connection_name: :anubis_redis, | ||
| redix_opts: [] # Optional Redix connection options | ||
|
|
||
| ## SSL/TLS Configuration | ||
|
|
||
| For Redis servers requiring TLS (like Upstash), pass SSL options via `:redix_opts`: | ||
|
|
||
| config :anubis_mcp, :session_store, | ||
| adapter: Anubis.Server.Session.Store.Redis, | ||
| redis_url: "rediss://default:password@host.upstash.io:6379", | ||
| redix_opts: [ | ||
| ssl: true, | ||
| socket_opts: [ | ||
| customize_hostname_check: [ | ||
| match_fun: :public_key.pkix_verify_hostname_match_fun(:https) | ||
| ] | ||
| ] | ||
| ] | ||
| ] | ||
|
|
||
| ## Features | ||
| ## Features | ||
|
|
||
| - Automatic session expiration using Redis TTL | ||
| - Last-write-wins semantics for session updates | ||
| - Connection pooling for high concurrency | ||
| - Namespace support for multi-tenant deployments | ||
| """ | ||
| - Automatic session expiration using Redis TTL | ||
| - Last-write-wins semantics for session updates | ||
| - Connection pooling for high concurrency | ||
| - Namespace support for multi-tenant deployments | ||
| """ |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
📝 P3: Document Redix as a required dependency
The moduledoc should explicitly state that Redix must be added to dependencies. Users reading the docs won't know they need to install it.
Suggest adding this section after line 4:
@moduledoc """
Redis-based session store implementation.
+## Dependencies
+
+Requires the `:redix` package:
+
+ # mix.exs
+ def deps do
+ [
+ {:redix, "~> 1.5"}
+ ]
+ end
+
Uses Redix for Redis communication and provides persistent session storage📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @moduledoc """ | |
| Redis-based session store implementation. | |
| Uses Redix for Redis communication and provides persistent session storage | |
| with automatic expiration and connection pooling. | |
| ## Configuration | |
| config :anubis_mcp, :session_store, | |
| adapter: Anubis.Server.Session.Store.Redis, | |
| redis_url: "redis://localhost:6379/0", | |
| pool_size: 10, | |
| ttl: 1_800_000, # 30 minutes in milliseconds | |
| namespace: "anubis:sessions", | |
| connection_name: :anubis_redis, | |
| redix_opts: [] # Optional Redix connection options | |
| ## SSL/TLS Configuration | |
| For Redis servers requiring TLS (like Upstash), pass SSL options via `:redix_opts`: | |
| config :anubis_mcp, :session_store, | |
| adapter: Anubis.Server.Session.Store.Redis, | |
| redis_url: "rediss://default:password@host.upstash.io:6379", | |
| redix_opts: [ | |
| ssl: true, | |
| socket_opts: [ | |
| customize_hostname_check: [ | |
| match_fun: :public_key.pkix_verify_hostname_match_fun(:https) | |
| ] | |
| ] | |
| ] | |
| ] | |
| ## Features | |
| ## Features | |
| - Automatic session expiration using Redis TTL | |
| - Last-write-wins semantics for session updates | |
| - Connection pooling for high concurrency | |
| - Namespace support for multi-tenant deployments | |
| """ | |
| - Automatic session expiration using Redis TTL | |
| - Last-write-wins semantics for session updates | |
| - Connection pooling for high concurrency | |
| - Namespace support for multi-tenant deployments | |
| """ | |
| @moduledoc """ | |
| Redis-based session store implementation. | |
| ## Dependencies | |
| Requires the `:redix` package: | |
| # mix.exs | |
| def deps do | |
| [ | |
| {:redix, "~> 1.5"} | |
| ] | |
| end | |
| Uses Redix for Redis communication and provides persistent session storage | |
| with automatic expiration and connection pooling. | |
| ## Configuration | |
| config :anubis_mcp, :session_store, | |
| adapter: Anubis.Server.Session.Store.Redis, | |
| redis_url: "redis://localhost:6379/0", | |
| pool_size: 10, | |
| ttl: 1_800_000, # 30 minutes in milliseconds | |
| namespace: "anubis:sessions", | |
| connection_name: :anubis_redis, | |
| redix_opts: [] # Optional Redix connection options | |
| ## SSL/TLS Configuration | |
| For Redis servers requiring TLS (like Upstash), pass SSL options via `:redix_opts`: | |
| config :anubis_mcp, :session_store, | |
| adapter: Anubis.Server.Session.Store.Redis, | |
| redis_url: "rediss://default:password@host.upstash.io:6379", | |
| redix_opts: [ | |
| ssl: true, | |
| socket_opts: [ | |
| customize_hostname_check: [ | |
| match_fun: :public_key.pkix_verify_hostname_match_fun(:https) | |
| ] | |
| ] | |
| ] | |
| ## Features | |
| - Automatic session expiration using Redis TTL | |
| - Last-write-wins semantics for session updates | |
| - Connection pooling for high concurrency | |
| - Namespace support for multi-tenant deployments | |
| """ |
🤖 Prompt for AI Agents
In lib/anubis/server/session/store/redis.ex around lines 3–42 (add after line
4), update the moduledoc to explicitly state that the Redix library must be
added to the project dependencies (mix.exs) and recommend specifying a minimum
compatible version; also note that Redix connection options (including
SSL/socket options for services like Upstash) should be provided via the
redix_opts config entry and, optionally, mention adding the named connection if
using connection pooling.
🚀 Want to release this? --- ## [0.17.0](v0.16.0...v0.17.0) (2025-12-09) ### Features * **redis:** add redix_opts for SSL/TLS support ([#59](#59)) ([33658ab](33658ab)) ### Bug Fixes * added server component description/0 callback ([#58](#58)) ([a094473](a094473)) * redix should be loaded ([#71](#71)) ([09b872f](09b872f)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added Redis SSL/TLS support via redix options. * **Bug Fixes** * Fixed server component description callback issue. * Fixed redix loading issue. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Problem <!-- what problem is the PR is trying to solve? --> ## Solution <!-- how is the PR solving the problem? --> ## Rationale <!-- why was it implemented the way it was? --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Improved conditional dependency handling for session storage to ensure graceful operation when optional libraries are unavailable. No behavioral changes for existing deployments. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
🚀 Want to release this? --- ## [0.17.0](v0.16.0...v0.17.0) (2025-12-09) ### Features * **redis:** add redix_opts for SSL/TLS support ([#59](#59)) ([3fd674a](3fd674a)) ### Bug Fixes * added server component description/0 callback ([#58](#58)) ([a094473](a094473)) * redix should be loaded ([#71](#71)) ([c352414](c352414)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added Redis SSL/TLS support via redix options. * **Bug Fixes** * Fixed server component description callback issue. * Fixed redix loading issue. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Problem
Solution
Rationale
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.