refactor: Migrate processes/services to devenv2#33
Merged
Conversation
📝 WalkthroughWalkthroughRefactors development environment to use environment-specific .env files, validate runtime ENVIRONMENT, allocate service HTTP ports dynamically via Nix, update readiness checks and tasks, remove legacy dev recipes, and make tests derive endpoints from configured VERDICT_URL/settings. Changes
Sequence Diagram(s)sequenceDiagram
participant Task as devenv task (gen-env / db tasks)
participant Postgres as Postgres DB
participant Mocks as Mock Services
participant App as verdict-app (Uvicorn)
Task->>Postgres: ensure DB name/port (DATABASE_NAME, DATABASE_PORT)
Task->>Mocks: read allocated mock ports -> generate `.env.{env}`
Task->>App: set VERDICT_URL from app allocated port
Postgres-->>Task: DB ready
Mocks-->>Task: Mocks ready (ready.http.get)
Task->>App: start Uvicorn with port from config
App->>App: readiness checked via ready.http.get on configured port
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 Tip CodeRabbit can enforce grammar and style rules using `languagetool`.Configure the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.gitignore:
- Around line 55-58: Add a rule to .gitignore to also ignore production env
files so secrets can't be committed: update the .gitignore patterns (near the
existing .env.development, .env.test, and .env.*.local entries) to include
.env.production (and optionally .env.*.production or .env.production.local) so
the repository ignores production environment files referenced by
verdict-backend/app/database_config.py.
In `@verdict-backend/app/database_config.py`:
- Around line 7-21: The module currently computes _environment and validates it
at import time (using _VALID_ENVIRONMENTS and _environment) which freezes
ENVIRONMENT and can raise during import; change this so ENVIRONMENT is resolved
and validated lazily when settings are requested (e.g., inside
get_database_settings() or DatabaseSettings.__init__/a factory), remove the
module-level os.getenv and the import-time raise, and build
DatabaseSettings.model_config.env_file (or pass env_file into BaseSettings)
using the environment value obtained at call time; ensure
get_database_settings() performs the validation against _VALID_ENVIRONMENTS and
uses that runtime value to construct the env_file list.
In `@verdict-backend/devenv.nix`:
- Around line 132-148: The db-migrate and db-setup tasks
(tasks."verdict-app:db-migrate" and tasks."verdict-app:db-setup") need to wait
for Postgres to be ready; add "devenv:processes:postgres" to each task's after
array so the tasks depend on the Postgres process being up before running
migrations or db reset/setup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d6f19a38-c2f1-472e-bcbd-9cfd8ae7f364
📒 Files selected for processing (7)
.gitignoremock-services/devenv.nixverdict-backend/app/database_config.pyverdict-backend/devenv.nixverdict-backend/justfileverdict-backend/tests/e2e/conftest.pyverdict-backend/tests/routes/test_ingestion.py
💤 Files with no reviewable changes (1)
- verdict-backend/justfile
There was a problem hiding this comment.
🧹 Nitpick comments (1)
verdict-backend/devenv.nix (1)
20-26: Consolidate duplicated env config into one source of truth.
ENVIRONMENT,DATABASE_NAME,DATABASE_PORT, andVERDICT_URLare defined twice (globalenvand.env.*generation). This can drift over time.Refactor sketch
+ # Single source of truth for shared runtime values + # (declare in outer let/in scope) + runtimeEnv = { + ENVIRONMENT = if config.devenv.isTesting then "test" else "development"; + DATABASE_NAME = if config.devenv.isTesting then "${database_name}_test" else database_name; + DATABASE_PORT = toString config.processes.postgres.ports.main.value; + VERDICT_URL = "http://localhost:${toString config.processes.verdict-app.ports.http.value}"; + }; + - env = { - DATABASE_NAME = if config.devenv.isTesting then "${database_name}_test" else database_name; - DATABASE_PORT = toString config.processes.postgres.ports.main.value; - ENVIRONMENT = if config.devenv.isTesting then "test" else "development"; - VERDICT_URL = "http://localhost:${toString config.processes.verdict-app.ports.http.value}"; - }; + env = runtimeEnv; tasks."verdict-app:gen-env" = { exec = '' cat > .env.${environment} <<EOF - ENVIRONMENT=${environment} - DATABASE_NAME=${db_name} - DATABASE_PORT=${db_port} + ENVIRONMENT=${runtimeEnv.ENVIRONMENT} + DATABASE_NAME=${runtimeEnv.DATABASE_NAME} + DATABASE_PORT=${runtimeEnv.DATABASE_PORT} ... - VERDICT_URL=http://localhost:${toString config.processes.verdict-app.ports.http.value} + VERDICT_URL=${runtimeEnv.VERDICT_URL} EOF ''; };Also applies to: 106-127
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@verdict-backend/devenv.nix` around lines 20 - 26, Duplicate environment definitions are present in the top-level env attribute and in the .env.* generation logic (keys: ENVIRONMENT, DATABASE_NAME, DATABASE_PORT, VERDICT_URL); consolidate them into a single source of truth by extracting those computed values into one place (e.g., a shared let-bound record or function) and have both the global env and the .env.* generators reference that single record (update usages that currently compute values inline such as the conditional on config.devenv.isTesting, toString config.processes.postgres.ports.main.value, and toString config.processes.verdict-app.ports.http.value); remove the duplicated inline computations so all changes flow from that one symbol and ensure the .env.* writer uses the same symbols for key names and values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@verdict-backend/devenv.nix`:
- Around line 20-26: Duplicate environment definitions are present in the
top-level env attribute and in the .env.* generation logic (keys: ENVIRONMENT,
DATABASE_NAME, DATABASE_PORT, VERDICT_URL); consolidate them into a single
source of truth by extracting those computed values into one place (e.g., a
shared let-bound record or function) and have both the global env and the .env.*
generators reference that single record (update usages that currently compute
values inline such as the conditional on config.devenv.isTesting, toString
config.processes.postgres.ports.main.value, and toString
config.processes.verdict-app.ports.http.value); remove the duplicated inline
computations so all changes flow from that one symbol and ensure the .env.*
writer uses the same symbols for key names and values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3d08bcc2-05c1-49cb-96b5-aadbaa74f9b8
📒 Files selected for processing (2)
.gitignoreverdict-backend/devenv.nix
✅ Files skipped from review due to trivial changes (1)
- .gitignore
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 by CodeRabbit
New Features
Refactor
Chores