Only create DB directory if using embedded DB #267
Conversation
If an external database is being used there is no need to create the local database files. This prevents the database files from being created unless the embedded database is being used.
WalkthroughThe changes update the Changes
Sequence Diagram(s)sequenceDiagram
participant S as Entrypoint.sh
participant DB as PostgreSQL Database
S->>S: Evaluate DATABASE_URL against connection string
alt Match Found
S->>S: Set DATABASE_EMBEDDED = true
else No Match
S->>S: DATABASE_EMBEDDED remains unset
end
S->>S: Check if DATABASE_DATA_DIR exists
alt DATABASE_DATA_DIR missing & DATABASE_EMBEDDED true
S->>S: Initialize database directory
end
S->>S: Check DATABASE_EMBEDDED condition for startup
alt DATABASE_EMBEDDED true
S->>DB: Start PostgreSQL Database
end
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
entrypoint.sh (3)
4-6: Set DATABASE_EMBEDDED Based on DATABASE_URL
The new conditional block correctly setsDATABASE_EMBEDDEDwhenDATABASE_URLexactly matches the embedded connection string. To improve clarity and avoid any future surprises, consider explicitly settingDATABASE_EMBEDDEDto"false"in an else clause so that its value is defined regardless of theDATABASE_URLvalue.
37-41: Initialization of DATABASE_DATA_DIR with Embedded DB
The condition now ensures that the database directory is only initialized when using the embedded database. However, in themkdir -pcommand, the$DATABASE_DATA_DIRvariable is not quoted while it is quoted in the subsequentchowncommand. To maintain consistency and avoid issues with paths that might include spaces, please quote$DATABASE_DATA_DIRin themkdircommand.Proposed diff:
- mkdir -p $DATABASE_DATA_DIR && chown -R postgres:postgres "$DATABASE_DATA_DIR" + mkdir -p "$DATABASE_DATA_DIR" && chown -R postgres:postgres "$DATABASE_DATA_DIR"🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 38-38: In POSIX sh, echo flags are undefined.
(SC3037)
136-142: Conditional Startup for Embedded PostgreSQL
The revised block correctly restricts the PostgreSQL startup to scenarios whereDATABASE_EMBEDDEDis"true". As a minor improvement, consider quoting$DATABASE_DATA_DIRin the command used bysu postgres -c "postgres -D $DATABASE_DATA_DIR"to handle potential spaces in the path.Proposed diff:
- su postgres -c "postgres -D $DATABASE_DATA_DIR" & + su postgres -c "postgres -D \"$DATABASE_DATA_DIR\"" &🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 139-139: In POSIX sh, echo flags are undefined.
(SC3037)
|
Thanks! Will merge tmwr when I get the chance to sanity check repro |
If an external database is being used there is no need to create the local database files. This prevents the database files from being created unless the embedded database is being used.
Summary by CodeRabbit