-
Notifications
You must be signed in to change notification settings - Fork 10
Fix/db connection pool limit #738
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,9 +11,16 @@ def get_engine(): | |
|
|
||
| # Configure connection pool settings | ||
| # For testing, we need more connections since tests run in parallel | ||
| pool_size = 20 if settings.ENVIRONMENT == "development" else 5 | ||
| max_overflow = 30 if settings.ENVIRONMENT == "development" else 10 | ||
|
|
||
| pool_size = 5 | ||
| max_overflow = 10 | ||
|
|
||
| if settings.ENVIRONMENT == "development": | ||
| pool_size = 20 | ||
| max_overflow = 30 | ||
| elif settings.ENVIRONMENT == "staging": | ||
| pool_size = 16 | ||
| # test it out | ||
| max_overflow = 0 | ||
|
Comment on lines
+14
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check if there are any other places where production pool sizing might be configured
# or documented, and verify the ENVIRONMENT values used in the codebase
echo "=== Checking ENVIRONMENT literal values ==="
rg -n "ENVIRONMENT" --type py -C2
echo ""
echo "=== Checking for any production-specific database config ==="
rg -n "production" --type py -C2
echo ""
echo "=== Checking for pool_size mentions elsewhere ==="
rg -n "pool_size" --type pyRepository: ProjectTech4DevAI/kaapi-backend Length of output: 4704 🏁 Script executed: cat -n backend/app/core/db.py | head -50Repository: ProjectTech4DevAI/kaapi-backend Length of output: 1994 🏁 Script executed: rg -n "from sqlalchemy|from sqlmodel|import.*Engine" backend/app/core/db.py -A2 -B2Repository: ProjectTech4DevAI/kaapi-backend Length of output: 397 🏁 Script executed: # Also check if there's any config documentation for production sizing
find backend -name "*.md" -o -name "*.txt" -o -name ".env*" | xargs grep -l "pool_size\|production" 2>/dev/null || echo "No matches found"Repository: ProjectTech4DevAI/kaapi-backend Length of output: 167 🏁 Script executed: grep -n "pool_size\|production\|database" backend/README.md -C3 | head -40Repository: ProjectTech4DevAI/kaapi-backend Length of output: 2556 🏁 Script executed: # Check what type create_engine returns (from sqlmodel/sqlalchemy)
python3 << 'EOF'
from sqlmodel import create_engine
import inspect
# Get the return type annotation
sig = inspect.signature(create_engine)
print(f"create_engine return type: {sig.return_annotation}")
EOFRepository: ProjectTech4DevAI/kaapi-backend Length of output: 195 🏁 Script executed: # Check how get_engine() is used to infer return type
rg -n "get_engine\(\)" backend --type py -A3 -B1Repository: ProjectTech4DevAI/kaapi-backend Length of output: 666 🏁 Script executed: # Check if Engine type is imported anywhere in the codebase
rg -n "Engine|from sqlalchemy" backend --type py | head -20Repository: ProjectTech4DevAI/kaapi-backend Length of output: 1695 🏁 Script executed: # Check for any logging in db.py and verify function signature requirements
cat -n backend/app/core/db.py | tail -30Repository: ProjectTech4DevAI/kaapi-backend Length of output: 1328 🏁 Script executed: # Verify Python version requirement in the codebase
rg -n "python_requires\|version" backend/pyproject.toml -A1 -B1 | head -20Repository: ProjectTech4DevAI/kaapi-backend Length of output: 57 🏁 Script executed: cat backend/pyproject.toml | head -30Repository: ProjectTech4DevAI/kaapi-backend Length of output: 900 Add type hint to The function at line 7 is missing a return type hint. Add
Currently, production has only 15 potential connections (5 + 10 overflow) compared to 50 for development and 16 for staging. 🤖 Prompt for AI Agents |
||
| return create_engine( | ||
| str(settings.SQLALCHEMY_DATABASE_URI), | ||
| pool_size=pool_size, | ||
|
|
||
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.
Stale comment: testing environment doesn't receive more connections.
The comment on line 13 states "For testing, we need more connections since tests run in parallel," but the
testingenvironment is not explicitly handled and falls through to the base defaults (pool_size=5,max_overflow=10). Either update the comment to reflect the actual behavior or add explicit handling for thetestingenvironment if parallel tests require larger pools.🤖 Prompt for AI Agents