feat(data): add configurable database provider via DATABASE_PROVIDER (#249)#490
Merged
nanotaboada merged 5 commits intomasterfrom May 3, 2026
Merged
Conversation
…249) Introduce DATABASE_PROVIDER env var to select the database engine at startup: sqlite (default, zero-infrastructure) or postgres (opt-in, requires Docker Compose postgres profile). Both providers use MigrateAsync() at startup for schema management. - Add Npgsql.EntityFrameworkCore.PostgreSQL 10.0.1 to Runtime deps - Rename AddDbContextPoolWithSqlite -> AddDbContextPool; reads DATABASE_PROVIDER and wires UseNpgsql or UseSqlite accordingly - Add ProviderSpecificMigrationsAssembly to filter EF Core migration discovery to the active provider's namespace at runtime - Add Migrations/Npgsql/ with PostgreSQL-typed migrations (uuid, boolean, timestamp with time zone) on separate timestamps to avoid MigrationAttribute.Id key collisions in the assembly scan - Add postgres Compose profile; api depends_on postgres with required: false so SQLite mode incurs no Docker dependency - Skip SQLite file-presence check in entrypoint.sh for postgres mode - Add .env.example; add .env to .gitignore - Add ADR-0014 superseding ADR-0003; update README and CHANGELOG Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds a configurable database provider via ChangesConfigurable Database Provider
Sequence Diagram(s)sequenceDiagram
participant DC as "docker compose"
participant Entrypoint as "entrypoint.sh"
participant API as "API (App)"
participant EF as "EF Core (Migrations)"
participant PG as "Postgres DB"
DC->>Entrypoint: start container (env: DATABASE_PROVIDER)
Entrypoint->>Entrypoint: normalize DATABASE_PROVIDER (default sqlite)
alt provider == postgres
Entrypoint->>API: launch app (Postgres mode)
API->>EF: configure UseNpgsql + ProviderSpecificMigrationsAssembly
API->>PG: connect & run MigrateAsync()
else provider == sqlite
Entrypoint->>Entrypoint: ensure SQLite file/dir
Entrypoint->>API: launch app (SQLite mode)
API->>EF: configure UseSqlite + ProviderSpecificMigrationsAssembly
API->>API: run MigrateAsync() against SQLite file
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Out-of-scope changesNo functional out-of-scope code changes were identified relative to the objectives in the linked issue. Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 👉 Get your free trial and get 200 agent minutes per Slack user (a $50 value). 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. Review rate limit: 0/1 reviews remaining, refill in 36 minutes and 36 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.example:
- Around line 6-9: Replace the real-looking password in the example env entries
with an unambiguous, non-deployable placeholder: update the DATABASE_URL value
to use a placeholder password (e.g., <your-postgres-password> or CHANGE_ME)
inside the connection string and set POSTGRES_PASSWORD to the same placeholder;
adjust any explanatory comment if needed so both DATABASE_URL and
POSTGRES_PASSWORD clearly signal they must be replaced before deployment (refer
to the DATABASE_URL and POSTGRES_PASSWORD entries to locate the changes).
In @.github/copilot-instructions.md:
- Around line 210-213: Update the README-style guidance that still claims the
repo is SQLite-only: change the overview and the "Never modify" section so they
acknowledge both SQLite and PostgreSQL options and mention the provider
switching behavior (refer to the existing "Switch database provider" text and
the ProviderSpecificMigrationsAssembly concept). Keep the step-by-step "Modify
schema" instructions but remove any SQLite-only wording and add that migrations
must be generated for both providers (Migrations/ and Migrations/Npgsql/) when
schema changes are made; also clarify that setting DATABASE_PROVIDER=postgres
plus DATABASE_URL enables Postgres. Ensure references to "Never modify" no
longer assert SQLite-only constraints and instead warn about editing
provider-specific migrations or migration assembly filtering.
In `@scripts/entrypoint.sh`:
- Around line 13-16: The script uses DATABASE_PROVIDER without normalizing case,
so comparisons (the if branch that checks "$DATABASE_PROVIDER" = "postgres") can
mismatch values like POSTGRES; change the assignment of DATABASE_PROVIDER in
scripts/entrypoint.sh to normalize to lowercase (consistent with
AddDbContextPool) before any branching, then use that normalized
DATABASE_PROVIDER for the if check and any subsequent logic; ensure the variable
name DATABASE_PROVIDER and the branch that checks "postgres" are updated to use
the lowercased value.
In
`@src/Dotnet.Samples.AspNetCore.WebApi/Extensions/ServiceCollectionExtensions.cs`:
- Around line 38-63: The switch currently treats any non-"postgres" value as
SQLite and lets empty DATABASE_URL through; update the logic around the provider
variable and the DATABASE_URL guard so mis-typed providers fail fast: trim and
normalize the provider (e.g., provider =
(Environment.GetEnvironmentVariable("DATABASE_PROVIDER") ?? "").Trim();) then
explicitly handle "postgres" with a strict DATABASE_URL check using
string.IsNullOrWhiteSpace before calling options.UseNpgsql(connectionString),
explicitly handle "sqlite" (or empty provider) to use UseSqlite, and for any
other non-empty provider throw an InvalidOperationException indicating an
unsupported DATABASE_PROVIDER value so typos (like "postgress") no longer
silently create a SQLite DB.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 807ab79e-2e2f-4d83-a180-bd931529bf5f
⛔ Files ignored due to path filters (9)
src/Dotnet.Samples.AspNetCore.WebApi/Migrations/Npgsql/20260409151000_InitialCreate.Designer.csis excluded by!**/Migrations/**,!**/*.Designer.cssrc/Dotnet.Samples.AspNetCore.WebApi/Migrations/Npgsql/20260409151000_InitialCreate.csis excluded by!**/Migrations/**src/Dotnet.Samples.AspNetCore.WebApi/Migrations/Npgsql/20260409151100_SeedStarting11.Designer.csis excluded by!**/Migrations/**,!**/*.Designer.cssrc/Dotnet.Samples.AspNetCore.WebApi/Migrations/Npgsql/20260409151100_SeedStarting11.csis excluded by!**/Migrations/**src/Dotnet.Samples.AspNetCore.WebApi/Migrations/Npgsql/20260409151200_SeedSubstitutes.Designer.csis excluded by!**/Migrations/**,!**/*.Designer.cssrc/Dotnet.Samples.AspNetCore.WebApi/Migrations/Npgsql/20260409151200_SeedSubstitutes.csis excluded by!**/Migrations/**src/Dotnet.Samples.AspNetCore.WebApi/Migrations/ProviderSpecificMigrationsAssembly.csis excluded by!**/Migrations/**src/Dotnet.Samples.AspNetCore.WebApi/packages.lock.jsonis excluded by!**/packages.lock.jsontest/Dotnet.Samples.AspNetCore.WebApi.Tests/packages.lock.jsonis excluded by!**/packages.lock.json
📒 Files selected for processing (15)
.env.example.github/copilot-instructions.md.gitignoreCHANGELOG.mdREADME.mdadr/0003-use-sqlite-for-data-storage.mdadr/0014-configurable-database-provider.mdadr/README.mdcompose.yamlscripts/entrypoint.shsrc/Dotnet.Samples.AspNetCore.WebApi/Dotnet.Samples.AspNetCore.WebApi.csprojsrc/Dotnet.Samples.AspNetCore.WebApi/Extensions/ServiceCollectionExtensions.cssrc/Dotnet.Samples.AspNetCore.WebApi/Program.cstest/Dotnet.Samples.AspNetCore.WebApi.Tests/Integration/PlayerWebApplicationTests.cstest/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/DatabaseFakes.cs
Add the six Migrations/Npgsql/ files to sonar.exclusions, sonar.coverage.exclusions, and sonar.cpd.exclusions for the same reasons the SQLite migration set is already excluded: EF Core- generated files with intentionally repetitive InsertData calls are not meaningful targets for duplication or coverage analysis. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…249) Address post-merge review findings on the configurable provider feature. No behavioral change for valid sqlite/postgres values. - ServiceCollectionExtensions: trim and normalize DATABASE_PROVIDER before the switch; add explicit "sqlite"/"" case; throw InvalidOperationException for unrecognized values so typos like "postgress" no longer silently fall through to SQLite; replace null-coalescing guard on DATABASE_URL with IsNullOrWhiteSpace - entrypoint.sh: normalize DATABASE_PROVIDER to lowercase via tr so POSTGRES, Postgres, etc. are handled consistently - copilot-instructions.md: update Overview to mention both providers; replace SQLite-only "Never modify" constraint with a warning about ProviderSpecificMigrationsAssembly namespace constants Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…ing (#249) Two bugs caught by Docker Compose sanity check of the PostgreSQL path. - Populate BuildTargetModel in Npgsql seed migration designer files so Npgsql's SQL generator can resolve column types when applying InsertData operations (SeedStarting11, SeedSubstitutes) - Suppress PendingModelChangesWarning for the postgres provider path via ConfigureWarnings; hand-crafted designer files cannot replicate Npgsql-injected runtime annotations (Relational:MaxIdentifierLength, UseIdentityByDefaultColumn), causing a false-positive that aborted MigrateAsync() at startup; BuildTargetModel is still populated so InsertData SQL generation continues to work correctly Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
nanotaboada
force-pushed
the
feat/249-configurable-database-provider
branch
from
45f4118 to
f937601
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Dotnet.Samples.AspNetCore.WebApi/Extensions/ServiceCollectionExtensions.cs (1)
32-35: ⚡ Quick winPrefer a feature-specific name over
AddDbContextPool.This reads like the EF Core framework method at the call site, but it now also resolves environment variables and swaps the migrations assembly. Renaming it to something like
AddPlayerDbContextorAddDatabaseProviderwould make the custom behavior explicit.As per coding guidelines:
src/**/Extensions/**/*.cs: "Follow the Add{Feature} naming convention (e.g. AddPlayerServices, AddRateLimiting)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Dotnet.Samples.AspNetCore.WebApi/Extensions/ServiceCollectionExtensions.cs` around lines 32 - 35, The method AddDbContextPool should be renamed to a feature-specific name (e.g., AddPlayerDbContext or AddDatabaseProvider) to avoid clashing with EF Core's AddDbContextPool and to reflect the custom behavior (env var resolution and migrations assembly swap); update the method declaration name and all callers to the new name, keep the existing signature (IServiceCollection Add... (this IServiceCollection services, IWebHostEnvironment environment)), and update any XML/documentation/comments to match the new Add{Feature} naming convention (e.g., AddPlayerDbContext).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/Dotnet.Samples.AspNetCore.WebApi/Extensions/ServiceCollectionExtensions.cs`:
- Around line 45-58: In the "postgres" branch (inside
ServiceCollectionExtensions.cs) the call options.UseNpgsql(connectionString)
needs to be replaced with the overload that enables transient-fault retries:
call options.UseNpgsql(connectionString, npgsqlOptions =>
npgsqlOptions.EnableRetryOnFailure(/* optionally maxRetryCount, maxRetryDelay,
errorCodes */)); keep the existing ConfigureWarnings call unchanged; this
attaches EF Core/Npgsql retry behavior so the db.Database.MigrateAsync() startup
call can survive transient connection blips.
---
Nitpick comments:
In
`@src/Dotnet.Samples.AspNetCore.WebApi/Extensions/ServiceCollectionExtensions.cs`:
- Around line 32-35: The method AddDbContextPool should be renamed to a
feature-specific name (e.g., AddPlayerDbContext or AddDatabaseProvider) to avoid
clashing with EF Core's AddDbContextPool and to reflect the custom behavior (env
var resolution and migrations assembly swap); update the method declaration name
and all callers to the new name, keep the existing signature (IServiceCollection
Add... (this IServiceCollection services, IWebHostEnvironment environment)), and
update any XML/documentation/comments to match the new Add{Feature} naming
convention (e.g., AddPlayerDbContext).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0de4c155-fa01-4e35-963c-50a2bae94c27
⛔ Files ignored due to path filters (2)
src/Dotnet.Samples.AspNetCore.WebApi/Migrations/Npgsql/20260409151100_SeedStarting11.Designer.csis excluded by!**/Migrations/**,!**/*.Designer.cssrc/Dotnet.Samples.AspNetCore.WebApi/Migrations/Npgsql/20260409151200_SeedSubstitutes.Designer.csis excluded by!**/Migrations/**,!**/*.Designer.cs
📒 Files selected for processing (2)
CHANGELOG.mdsrc/Dotnet.Samples.AspNetCore.WebApi/Extensions/ServiceCollectionExtensions.cs
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
- Rename AddDbContextPool -> AddPlayerDbContext to follow the Add{Feature}
naming convention and avoid shadowing EF Core's own AddDbContextPool;
update call site in Program.cs accordingly
- Add EnableRetryOnFailure() to the Npgsql path so MigrateAsync() and
subsequent queries survive transient connection blips at startup
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
nanotaboada
force-pushed
the
feat/249-configurable-database-provider
branch
from
6e252af to
9a513b4
Compare
|
Owner
Author
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DATABASE_PROVIDERenv var (sqlitedefault,postgresopt-in) to select the database engine at startup without code changesProviderSpecificMigrationsAssemblyto filter EF Core migration discovery to the active provider's namespace at runtime, with separateMigrations/Npgsql/migrations using PostgreSQL-typed columns (uuid,boolean,timestamp with time zone)postgresDocker Compose profile withdepends_on: required: falseso SQLite mode incurs no Docker dependencyTest plan
dotnet build --configuration Release— succeedsdotnet test --settings .runsettings— 64/64 passeddotnet csharpier --check .— cleanNpgsql.EntityFrameworkCore.PostgreSQLfrom Development to RuntimeItemGroup)Closes #249
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores
Tests