Use cached settings factory for engine URL#54
Conversation
WalkthroughReplaced direct Settings() instantiation with get_settings() inside backend/api/db.py:get_engine_url(), keeping existing environment variable override and sqlite/Postgres URL normalization logic unchanged. No public signatures or broader control flow altered. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant DB as db.get_engine_url
participant CFG as get_settings
participant ENV as os.environ
Caller->>DB: get_engine_url()
DB->>CFG: get_settings()
CFG-->>DB: Settings instance
DB->>ENV: Check DATABASE_URL
alt DATABASE_URL set
DB-->>Caller: Return ENV DATABASE_URL
else Not set
DB->>DB: Build URL from settings
opt Normalize
DB->>DB: sqlite path handling / Postgres normalization
end
DB-->>Caller: Return computed URL
end
note over DB,CFG: Change: use get_settings() instead of Settings()
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)✅ Unit Test PR creation complete.
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
backend/api/db.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/api/db.py (2)
backend/api/config.py (1)
get_settings(50-53)backend/api/security.py (1)
get_settings(19-21)
| def get_engine_url() -> str: | ||
| """Resolve the database URL from env or settings with sane defaults.""" | ||
| settings = Settings() | ||
| settings = get_settings() |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
Verify that caching is actually implemented in get_settings().
The change correctly uses the factory function, but the provided code snippet from backend/api/config.py shows get_settings() simply returning Settings() without visible caching mechanism. Please verify that get_settings() actually implements caching (e.g., via @lru_cache() decorator) to achieve the stated goal of "reusing the shared cached settings instance."
Additionally, note that get_settings() appears in both backend/api/config.py and backend/api/security.py. Consider consolidating these implementations to avoid duplication and potential inconsistencies.
Run the following script to verify the caching implementation:
🏁 Script executed:
#!/bin/bash
# Description: Check if get_settings() uses caching decorator and identify duplicate implementations
# Check for caching decorators on get_settings() in config.py
echo "=== Checking config.py for caching implementation ==="
rg -A 5 -B 3 'def get_settings' backend/api/config.py
# Check for caching decorators on get_settings() in security.py
echo "=== Checking security.py for caching implementation ==="
rg -A 5 -B 3 'def get_settings' backend/api/security.py
# Search for lru_cache imports in both files
echo "=== Checking for lru_cache imports ==="
rg 'from functools import.*lru_cache' backend/api/config.py backend/api/security.pyLength of output: 576
Consolidate duplicate get_settings() implementations
config.py’s get_settings() is correctly cached via @lru_cache(). Please remove or unify the duplicate, uncached get_settings() in backend/api/security.py to avoid inconsistencies.
🤖 Prompt for AI Agents
In backend/api/db.py around line 32, you call get_settings() directly but there
is a duplicate, uncached get_settings() implementation in
backend/api/security.py; remove the duplicate implementation in security.py and
instead import the cached get_settings from the central config module (where
@lru_cache() is applied), update any imports/usages in both
backend/api/security.py and backend/api/db.py to use that single cached
function, and run tests/linters to ensure no unresolved imports remain.
|
Note Unit test generation is an Early Access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
Note Docstrings generation - SKIPPED |
Docstrings generation was requested by @shayancoin. * #54 (comment) The following files were modified: * `backend/api/db.py`
…#58) Docstrings generation was requested by @shayancoin. * #54 (comment) The following files were modified: * `backend/api/db.py` Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
✅ UTG Post-Process Complete No new issues were detected in the generated code and all check runs have completed. The unit test generation process has completed successfully. |
|
Creating a PR to put the unit tests in... The changes have been created in this pull request: View PR |
1 participant
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68f06252fe008330a1ce7e40e90128f0
Summary by CodeRabbit