-
-
Notifications
You must be signed in to change notification settings - Fork 256
Add redis parameter to bit Boilerplate (#11937) #11938
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
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit 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. WalkthroughThis PR adds Redis support to the Bit.Boilerplate template by introducing a Changes
Sequence Diagram(s)sequenceDiagram
participant Aspire as Aspire Host
participant Redis1 as redis-cache<br/>(Ephemeral)
participant Redis2 as redis-persistent<br/>(Persistent)
participant API as Server API
participant FusionCache
participant SignalR
participant Hangfire
participant DistLock as Distributed<br/>Locking
Aspire->>Redis1: Create ephemeral instance
Aspire->>Redis2: Create persistent instance<br/>(with AOF + data volume)
API->>Redis1: Register redis-cache multiplexer
API->>Redis2: Register redis-persistent multiplexer
FusionCache->>Redis1: Connect for L2 cache
FusionCache->>Redis1: Connect for backplane
SignalR->>Redis1: Connect for backplane<br/>(scale-out)
Hangfire->>Redis2: Connect for job storage<br/>(RedisStorageOptions)
DistLock->>Redis2: Connect for distributed locks
rect rgba(100, 150, 200, 0.1)
note over Redis1: In-memory, regenerable data
note over Redis2: Persistent, critical data
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Shared/Boilerplate.Server.Shared.csproj (1)
26-28: Same condition inconsistency as noted in AppHost.csproj.These conditions also use
OR '$(redis)' == '', which conflicts with thedefaultValue: "false"in template.json. See the previous review comment on Boilerplate.Server.AppHost.csproj for details.
🧹 Nitpick comments (2)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/appsettings.json (1)
9-10: Add explanatory comments for Redis connection strings.For consistency with the Api project's appsettings.json (lines 27-30), consider adding comments that explain the purpose of each Redis instance:
🔎 Suggested addition
- "redis-cache": "localhost:6379", - "redis-persistent": "localhost:6380", + "redis-cache": "localhost:6379", + "redis-cache_Comment": "Redis for FusionCache L2 distributed cache layer and SignalR backplane - no persistence needed, acts as fast ephemeral cache", + "redis-persistent": "localhost:6380", + "redis-persistent_Comment": "Redis for Hangfire background jobs and distributed locking - requires persistence for durability",src/Templates/Boilerplate/Bit.Boilerplate/.github/copilot-instructions.md (1)
25-27: Minor capitalization inconsistency in Redis description.The description uses lowercase "hangfire" and "signalr", while line 28 uses "Hangfire" and line 23 uses "SignalR" with proper capitalization.
🔎 Suggested fix
-* **Redis**: Distributed caching storage and backplane, hangfire job storage, signalr backplane and distributed lock. +* **Redis**: Distributed caching storage and backplane, Hangfire job storage, SignalR backplane, and distributed lock.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (14)
.github/workflows/bit.full.ci.ymlsrc/Templates/Boilerplate/Bit.Boilerplate/.docs/14- Response Caching System.mdsrc/Templates/Boilerplate/Bit.Boilerplate/.github/copilot-instructions.mdsrc/Templates/Boilerplate/Bit.Boilerplate/.template.config/ide.host.jsonsrc/Templates/Boilerplate/Bit.Boilerplate/.template.config/template.jsonsrc/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.propssrc/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Boilerplate.Server.Api.csprojsrc/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cssrc/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/appsettings.jsonsrc/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.AppHost/Boilerplate.Server.AppHost.csprojsrc/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.AppHost/Program.cssrc/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Shared/Boilerplate.Server.Shared.csprojsrc/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Shared/Extensions/WebApplicationBuilderExtensions.cssrc/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/appsettings.json
🧰 Additional context used
🧬 Code graph analysis (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs (2)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Extensions/IConfigurationExtensions.cs (1)
GetRequiredConnectionString(5-11)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Models/Identity/Role.cs (1)
Role(3-7)
🪛 Biome (2.1.2)
src/Templates/Boilerplate/Bit.Boilerplate/.template.config/template.json
[error] 176-176: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 176-176: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 176-182: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 182-182: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/appsettings.json
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 29-29: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 29-29: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 29-29: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 29-29: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 30-30: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 30-30: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 30-30: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 30-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
⏰ 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). (1)
- GitHub Check: build Bit.Templates
🔇 Additional comments (17)
.github/workflows/bit.full.ci.yml (3)
138-138: Testing strategy: SQL Server path without Redis or Aspire.The SQL Server test path deliberately excludes both
--aspireand--redis, while the PostgreSQL path includes both. This provides good coverage by testing multiple configuration combinations.
232-238: Comprehensive Redis configuration coverage in sample builds.The two sample configurations effectively test:
- Configuration 1 (line 232): Redis enabled with Aspire and full feature set
- Configuration 2 (line 238): Redis explicitly disabled with Aspire enabled
This validates both the presence and absence of Redis functionality, ensuring the template parameter works correctly in different scenarios.
73-73: The--redisparameter is valid and properly supported in the template.The addition of
--redisto the PostgreSQL template invocation is correct. The template explicitly defines this boolean parameter, and the Aspire orchestration (viaProgram.csin AppHost) properly configures two Redis instances withbuilder.AddRedis()when the flag is enabled. The conditional compilation (//#if(redis == true)) ensures Redis is only included when the parameter is set.src/Templates/Boilerplate/Bit.Boilerplate/.template.config/template.json (1)
176-182: LGTM! Redis parameter definition is correct.The new
redisparameter follows the established pattern for boolean template symbols and is appropriately placed in the symbols section. ThedefaultValue: "false"is suitable for an optional infrastructure component.src/Templates/Boilerplate/Bit.Boilerplate/.template.config/ide.host.json (1)
17-19: LGTM! Symbol registration is correct.The
redissymbolInfo entry is properly registered and will be exposed in the IDE template UI.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/appsettings.json (1)
26-31: LGTM! Redis connection strings are properly configured.The configuration correctly:
- Uses separate ports (6379 for cache, 6380 for persistent) for the two Redis instances
- Provides clear comments explaining the purpose of each instance
- Uses the correct conditional directive
//#if (redis == true)without the problematic empty-string fallbackNote: The Biome static analysis errors are false positives caused by the template preprocessing directives.
src/Templates/Boilerplate/Bit.Boilerplate/.docs/14- Response Caching System.md (1)
497-516: No Redis Infrastructure duplication exists in the document.The search found only one instance of the "## Redis Infrastructure" section header at line 497. The content itself is well-written and clearly explains the two-instance architecture and separation rationale.
Likely an incorrect or invalid review comment.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Shared/Extensions/WebApplicationBuilderExtensions.cs (2)
49-55: Clean separation of Redis instances for different use cases.The design correctly separates persistent Redis (for Hangfire, distributed locking) from ephemeral Redis (for caching). Using
AddRedisClientfor persistent (non-keyed) andAddKeyedRedisClientfor cache allows downstream code to resolve them appropriately viaGetRequiredService<IConnectionMultiplexer>()vsGetRequiredKeyedService<IConnectionMultiplexer>("redis-cache").
60-74: FusionCache Redis configuration looks correct.The distributed cache and backplane correctly use the keyed
"redis-cache"multiplexer. Theasync () =>lambdas returning a synchronous value are valid for theFunc<Task<IConnectionMultiplexer>>delegate signature.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Boilerplate.Server.Api.csproj (1)
73-75: Redis package references are correctly conditioned.The conditional logic properly gates Redis packages and correctly requires both
$(redis)and$(signalR)for the SignalR Redis backplane package.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.AppHost/Program.cs (2)
8-25: Well-designed Redis infrastructure with appropriate persistence settings.The separation between ephemeral cache and persistent storage is correctly implemented:
redis-cache: No persistence, suitable for caching and SignalR backplaneredis-persistent: AOF withappendfsync=alwaysandnoevictionpolicy, appropriate for Hangfire jobs and distributed locksThe comment on line 21 helpfully notes that
appendfsync=alwayscan be temporarily disabled for bulk operations if needed.
106-109: Redis references correctly wired withWaitFor.Both Redis instances are referenced with
WaitFor, ensuring the servers won't start until Redis is available. This prevents startup failures from missing Redis connections.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs (4)
168-179: Redis distributed lock correctly uses the persistent Redis instance.
GetRequiredService<IConnectionMultiplexer>()resolves the non-keyed registration fromAddRedisClient("redis-persistent"), ensuring locks use the persistent Redis with AOF durability.
255-262: SignalR Redis backplane appropriately uses the ephemeral cache instance.Using
redis-cachefor the SignalR backplane is correct — pub/sub messages don't require persistence. Thesignalr:channel prefix properly namespaces the messages.
527-567: Hangfire Redis configuration is well-structured.Good isolation strategy using
Db = 1to separate Hangfire from distributed locks (which use the defaultDb = 0). Thehangfire:prefix provides additional namespacing. The refactoredAddHangfireoverload correctly injectsIServiceProviderfor Redis connection resolution.
585-585: Explicit namespace qualification forRoleis a safe choice.Using
Models.Identity.Roleremoves any potential ambiguity, though the namespace is already imported. This is a minor stylistic improvement.src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.props (1)
110-116: Redis package versions are consistent with the project.The Aspire packages use 13.1.0, ASP.NET Core packages use 10.0.1, and FusionCache packages use 2.5.0 — all matching their respective families elsewhere in this file.
...late/Bit.Boilerplate/src/Server/Boilerplate.Server.AppHost/Boilerplate.Server.AppHost.csproj
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds Redis support to the Bit Boilerplate template through a new configurable redis parameter. The implementation introduces two separate Redis instances: a persistent Redis instance for Hangfire background jobs and distributed locking, and an ephemeral cache-focused Redis instance for FusionCache L2 distributed caching and SignalR backplane. The parameter defaults to false to maintain backward compatibility.
Key changes:
- Added a new template parameter
redis(defaulting tofalse) for optional Redis infrastructure - Configured two Redis instances in Aspire AppHost: persistent Redis with AOF for durability and ephemeral Redis for caching
- Updated service registrations to use Redis for Hangfire storage, SignalR backplane, distributed locking, and FusionCache distributed cache when enabled
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
.template.config/template.json |
Added redis template parameter with default value of false |
.template.config/ide.host.json |
Registered the redis parameter for IDE integration |
.github/copilot-instructions.md |
Added documentation about Redis usage for caching, backplane, job storage, and distributed locks |
Server.AppHost/Program.cs |
Configured two Redis instances with appropriate persistence settings and added project references |
Server.AppHost/Boilerplate.Server.AppHost.csproj |
Added conditional package reference for Aspire.Hosting.Redis |
Server.Shared/Extensions/WebApplicationBuilderExtensions.cs |
Registered Redis clients and configured FusionCache with Redis distributed cache and backplane |
Server.Shared/Boilerplate.Server.Shared.csproj |
Added conditional package references for Redis-related libraries |
Server.Api/Program.Services.cs |
Configured Hangfire with Redis storage, SignalR with Redis backplane, and Redis-based distributed locking |
Server.Api/Boilerplate.Server.Api.csproj |
Added conditional package references for Redis functionality |
Server.Api/appsettings.json |
Added connection string entries for both Redis instances with documentation comments |
Server.Web/appsettings.json |
Added connection string entries for both Redis instances |
Directory.Packages.props |
Added Redis-related package versions and minor Scalar.AspNetCore version update |
.github/workflows/bit.full.ci.yml |
Updated CI workflow to test Redis parameter in sample configurations |
closes #11937
Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.