console: create db if not exists during init#234
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughReplaces build-time startup in the floconsole Dockerfile with an entrypoint-driven init script that ensures the PostgreSQL database exists and then launches the Uvicorn server; GitHub Actions workflows were extended to push images to AWS ECR in addition to GCP. Changes
Sequence DiagramsequenceDiagram
participant Container as Docker Container
participant Init as console-server-init.sh
participant Postgres as PostgreSQL
participant Python as Python Runtime / uvicorn
Container->>Init: ENTRYPOINT (execute script)
activate Init
Init->>Init: set -e, prepend venv to PATH
Init->>Postgres: connect using env vars
activate Postgres
alt DB exists
Postgres-->>Init: exists
else DB missing
Init->>Postgres: CREATE DATABASE
Postgres-->>Init: created
end
deactivate Postgres
Init->>Python: run `uv run server.py`
deactivate Init
activate Python
Python-->>Container: server running (accept requests)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
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 |
![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
🧹 Nitpick comments (1)
wavefront/server/docker/floconsole.Dockerfile (1)
20-23: Improve Docker cache reuse by reordering script copy after dependency sync.When the script on lines 20–21 changes, line 23's expensive
uv synclayer is unnecessarily invalidated and re-executed. Reorder to runuv syncfirst, then useCOPY --chmod=755to copy the script; this prevents cache invalidation when script contents change. Your CI setup (docker/setup-buildx-action@v3) supports BuildKit and COPY --chmod.Proposed refactor
-COPY wavefront/server/scripts/console-server-init.sh /app/scripts/console-server-init.sh -RUN chmod +x /app/scripts/console-server-init.sh - RUN uv sync --package floconsole --frozen --no-dev +COPY --chmod=755 wavefront/server/scripts/console-server-init.sh /app/scripts/console-server-init.sh + ENTRYPOINT ["/app/scripts/console-server-init.sh"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wavefront/server/docker/floconsole.Dockerfile` around lines 20 - 23, Reorder the Dockerfile steps so the expensive dependency installation RUN uv sync --package floconsole --frozen --no-dev runs before copying the script, and replace the current COPY + chmod sequence with a single COPY --chmod=755 /app/scripts/console-server-init.sh (i.e. move the RUN uv sync line above the COPY and use COPY --chmod=755 to make /app/scripts/console-server-init.sh executable) to avoid invalidating the uv sync cache when the script changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@wavefront/server/scripts/console-server-init.sh`:
- Around line 15-22: Add a connection timeout to psycopg2.connect by passing
connect_timeout (e.g., connect_timeout=5) to avoid hangs, and replace the
SELECT-then-CREATE pattern with a safe try/except around the CREATE DATABASE
call: use psycopg2.sql.Identifier to build the identifier for dbname when
calling cur.execute(sql.SQL('CREATE DATABASE {}').format(Identifier(dbname)))
and catch the specific duplicate-database error
(psycopg2.errors.DuplicateDatabase or the appropriate DatabaseError SQLSTATE) to
ignore it while re-raising other exceptions; ensure conn.autocommit remains True
so CREATE runs outside transactions.
---
Nitpick comments:
In `@wavefront/server/docker/floconsole.Dockerfile`:
- Around line 20-23: Reorder the Dockerfile steps so the expensive dependency
installation RUN uv sync --package floconsole --frozen --no-dev runs before
copying the script, and replace the current COPY + chmod sequence with a single
COPY --chmod=755 /app/scripts/console-server-init.sh (i.e. move the RUN uv sync
line above the COPY and use COPY --chmod=755 to make
/app/scripts/console-server-init.sh executable) to avoid invalidating the uv
sync cache when the script changes.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
wavefront/server/docker/floconsole.Dockerfilewavefront/server/scripts/console-server-init.sh
| conn = psycopg2.connect(host=host, user=user, password=password, dbname='postgres') | ||
| conn.autocommit = True | ||
| cur = conn.cursor() | ||
| cur.execute(\"SELECT 1 FROM pg_database WHERE datname = %s\", (dbname,)) | ||
|
|
||
| if not cur.fetchone(): | ||
| cur.execute('CREATE DATABASE \"' + dbname.replace('\"', '\"\"') + '\"') | ||
| print('Database created') |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and read the relevant lines
cat -n wavefront/server/scripts/console-server-init.sh | head -80Repository: rootflo/wavefront
Length of output: 1022
🏁 Script executed:
# Check for psycopg2 version in requirements/dependencies
fd -t f -e txt -e pip -e in requirements
fd -t f -e py -e txt setup.py setup.cfg pyproject.toml
rg -l "psycopg2" --type py --type txtRepository: rootflo/wavefront
Length of output: 218
🏁 Script executed:
# Find requirements files
fd requirements
fd "\.txt$|\.txt$|setup\.py" | head -20
rg -l "psycopg2" | head -10Repository: rootflo/wavefront
Length of output: 322
🏁 Script executed:
# Check the floconsole pyproject.toml for psycopg2
cat wavefront/server/apps/floconsole/pyproject.toml | rg -A 5 -B 5 "psycopg2|dependencies"Repository: rootflo/wavefront
Length of output: 625
🌐 Web query:
psycopg2 connect_timeout parameter documentation
💡 Result:
connect_timeout in psycopg2 is not a psycopg2-specific option; it’s a libpq connection parameter that psycopg2 passes through.
-
How to use (keyword arg):
psycopg2.connect(host="...", dbname="...", user="...", password="...", connect_timeout=10)
Psycopg2 passes “any other keyword parameter” through to the underlying client library (libpq), so
connect_timeoutis accepted this way. [2] -
How to use (DSN string):
psycopg2.connect("host=... dbname=... user=... password=... connect_timeout=10")
Libpq documents
connect_timeoutas the maximum time to wait while connecting, in seconds; 0/negative/omitted means wait indefinitely, and the timeout is applied per host when multiple hosts are provided. [1]
Docs:
- libpq connection parameters (includes
connect_timeout) [1] - psycopg2
connect()parameters / pass-through behavior [2]
Sources:
[1] PostgreSQL libpq connection parameters documentation (libpq-connect / parameter keywords) (postgresql.org)
[2] Psycopg2 connect() documentation (“any other keyword parameter will be passed to the underlying client library”) (access.crunchydata.com)
Citations:
- 1: https://www.postgresql.org/docs/current/libpq-connect.html?utm_source=openai
- 2: https://access.crunchydata.com/documentation/psycopg2/2.7.7/module.html?utm_source=openai
🌐 Web query:
psycopg2 sql module Identifier DuplicateDatabase exception
💡 Result:
In psycopg2 you can’t pass a database name as a %s parameter (placeholders are for values, not identifiers). For CREATE DATABASE, compose the identifier with psycopg2.sql.Identifier, then catch psycopg2.errors.DuplicateDatabase (SQLSTATE 42P04, base class ProgrammingError). [1][2]
import psycopg2
from psycopg2 import sql, errors
db_name = "mydb"
conn = psycopg2.connect("dbname=postgres user=postgres password=...")
conn.autocommit = True # CREATE DATABASE must not run inside a transaction
cur = conn.cursor()
try:
cur.execute(sql.SQL("CREATE DATABASE {}").format(sql.Identifier(db_name)))
except errors.DuplicateDatabase:
# db already exists (42P04)
pass
finally:
cur.close()
conn.close()If you need a qualified identifier elsewhere (e.g., "schema"."table"), pass multiple parts: sql.Identifier("schema", "table"). [1]
Sources: psycopg2 SQL composition / Identifier docs [1]; psycopg2 SQLSTATE-to-exception mapping showing 42P04 -> DuplicateDatabase [2].
Harden DB creation against concurrent startup and connection hangs.
Line 15 lacks a connection timeout, and lines 18–22 use a check-then-create sequence vulnerable to race conditions when multiple containers start concurrently. One contender can fail with a duplicate database error.
Add connect_timeout parameter to psycopg2.connect(), replace the check-then-create pattern with exception handling, and use psycopg2.sql.Identifier for safe identifier composition:
Proposed fix
-python3 -c "
-import psycopg2, os
+python3 -c "
+import os
+import psycopg2
+from psycopg2 import sql, errors
host = os.environ['CONSOLE_DB_HOST']
user = os.environ['CONSOLE_DB_USERNAME']
password = os.environ['CONSOLE_DB_PASSWORD']
dbname = os.environ['CONSOLE_DB_NAME']
+connect_timeout = int(os.getenv('CONSOLE_DB_CONNECT_TIMEOUT', '5'))
-conn = psycopg2.connect(host=host, user=user, password=password, dbname='postgres')
+conn = psycopg2.connect(
+ host=host,
+ user=user,
+ password=password,
+ dbname='postgres',
+ connect_timeout=connect_timeout,
+)
conn.autocommit = True
cur = conn.cursor()
-cur.execute(\"SELECT 1 FROM pg_database WHERE datname = %s\", (dbname,))
-
-if not cur.fetchone():
- cur.execute('CREATE DATABASE \"' + dbname.replace('\"', '\"\"') + '\"')
- print('Database created')
-else:
- print('Database already exists, skipping')
+try:
+ cur.execute(sql.SQL('CREATE DATABASE {}').format(sql.Identifier(dbname)))
+ print('Database created')
+except errors.DuplicateDatabase:
+ print('Database already exists, skipping')
+finally:
+ cur.close()
conn.close()
"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| conn = psycopg2.connect(host=host, user=user, password=password, dbname='postgres') | |
| conn.autocommit = True | |
| cur = conn.cursor() | |
| cur.execute(\"SELECT 1 FROM pg_database WHERE datname = %s\", (dbname,)) | |
| if not cur.fetchone(): | |
| cur.execute('CREATE DATABASE \"' + dbname.replace('\"', '\"\"') + '\"') | |
| print('Database created') | |
| import os | |
| import psycopg2 | |
| from psycopg2 import sql, errors | |
| host = os.environ['CONSOLE_DB_HOST'] | |
| user = os.environ['CONSOLE_DB_USERNAME'] | |
| password = os.environ['CONSOLE_DB_PASSWORD'] | |
| dbname = os.environ['CONSOLE_DB_NAME'] | |
| connect_timeout = int(os.getenv('CONSOLE_DB_CONNECT_TIMEOUT', '5')) | |
| conn = psycopg2.connect( | |
| host=host, | |
| user=user, | |
| password=password, | |
| dbname='postgres', | |
| connect_timeout=connect_timeout, | |
| ) | |
| conn.autocommit = True | |
| cur = conn.cursor() | |
| try: | |
| cur.execute(sql.SQL('CREATE DATABASE {}').format(sql.Identifier(dbname))) | |
| print('Database created') | |
| except errors.DuplicateDatabase: | |
| print('Database already exists, skipping') | |
| finally: | |
| cur.close() | |
| conn.close() |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wavefront/server/scripts/console-server-init.sh` around lines 15 - 22, Add a
connection timeout to psycopg2.connect by passing connect_timeout (e.g.,
connect_timeout=5) to avoid hangs, and replace the SELECT-then-CREATE pattern
with a safe try/except around the CREATE DATABASE call: use
psycopg2.sql.Identifier to build the identifier for dbname when calling
cur.execute(sql.SQL('CREATE DATABASE {}').format(Identifier(dbname))) and catch
the specific duplicate-database error (psycopg2.errors.DuplicateDatabase or the
appropriate DatabaseError SQLSTATE) to ignore it while re-raising other
exceptions; ensure conn.autocommit remains True so CREATE runs outside
transactions.
* console: create db if not exists during init * chore: push images to ECR
2 participants
Summary by CodeRabbit