chore: Create appsmith postgres DB to be used by internal processes #36670
Conversation
abhvsn
requested review from
srix
and removed request for
sharat87 and
pratapaprasanna
WalkthroughThe changes in this pull request involve modifications to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
Documentation and Community
|
github-actions
bot
added
the
skip-changelog
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| DB_EXISTS=$(su postgres -c "env PATH='$PATH' psql -tAc \"SELECT 1 FROM pg_database WHERE datname='${APPSMITH_PG_DATABASE}'\"") | ||
|
|
||
| if [[ "$DB_EXISTS" != "1" ]]; then | ||
| su postgres -c "env PATH='$PATH' psql -c 'CREATE DATABASE ${APPSMITH_PG_DATABASE}'" |
There was a problem hiding this comment.
Correct the variable expansion in psql command to avoid unintended literals
Remember, when scripting in Bash, single quotes prevent variable expansion, which means ${APPSMITH_PG_DATABASE} won't be replaced with its actual value inside single quotes. To ensure the database name is correctly used, let's use double quotes around the SQL command. This way, the variable will be expanded properly, and the database will be created with the correct name.
Apply this diff to fix the variable expansion:
- su postgres -c "env PATH='$PATH' psql -c 'CREATE DATABASE ${APPSMITH_PG_DATABASE}'"
+ su postgres -c "env PATH='$PATH' psql -c \"CREATE DATABASE ${APPSMITH_PG_DATABASE}\""📝 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.
| su postgres -c "env PATH='$PATH' psql -c 'CREATE DATABASE ${APPSMITH_PG_DATABASE}'" | |
| su postgres -c "env PATH='$PATH' psql -c \"CREATE DATABASE ${APPSMITH_PG_DATABASE}\"" |
There was a problem hiding this comment.
I echo with this suggestion
There was a problem hiding this comment.
@pratapaprasanna, I'm glad to hear that you agree with the suggestion. If there's anything else you'd like assistance with, please don't hesitate to ask.
(_/)
( ^‿^)
/❀❀\
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- deploy/docker/fs/opt/appsmith/entrypoint.sh (3 hunks)
🔇 Additional comments (2)
deploy/docker/fs/opt/appsmith/entrypoint.sh (2)
9-9: Well done, class! This new variable is a great addition.The new environment variable
APPSMITH_PG_DATABASEis set correctly to "appsmith". This aligns perfectly with our lesson on establishing a centralized PostgreSQL database. Keep up the good work!
436-436: A+ for integrating our new function!Excellent job incorporating the
create_appsmith_pg_dbfunction into theinit_postgresfunction. This addition ensures that our Appsmith database is created right after the PostgreSQL data folder is initialized. It's like adding the final piece to complete our database setup puzzle!
| create_appsmith_pg_db() { | ||
| POSTGRES_DB_PATH=$1 | ||
| # Start the postgres , wait for it to be ready and create a appsmith db | ||
| su postgres -c "env PATH='$PATH' pg_ctl -D $POSTGRES_DB_PATH -l $POSTGRES_DB_PATH/logfile start" | ||
| echo "Waiting for Postgres to start" | ||
| until su postgres -c "env PATH='$PATH' pg_isready -d postgres"; do | ||
| tlog "Waiting for Postgres to be ready..." | ||
| sleep 1 | ||
| done | ||
| # Check if the appsmith DB is present | ||
| DB_EXISTS=$(su postgres -c "env PATH='$PATH' psql -tAc \"SELECT 1 FROM pg_database WHERE datname='${APPSMITH_PG_DATABASE}'\"") | ||
|
|
||
| if [[ "$DB_EXISTS" != "1" ]]; then | ||
| su postgres -c "env PATH='$PATH' psql -c 'CREATE DATABASE ${APPSMITH_PG_DATABASE}'" | ||
| else | ||
| echo "Database ${APPSMITH_PG_DATABASE} already exists." | ||
| fi | ||
| su postgres -c "env PATH='$PATH' pg_ctl -D $POSTGRES_DB_PATH stop" | ||
| } |
There was a problem hiding this comment.
Excellent work on this new function, students! Let's make one small improvement.
The create_appsmith_pg_db function is well-structured and follows good database management practices. It's like a well-organized lesson plan! However, we need to make a small correction in our SQL syntax. Remember, when using variables in SQL, we need to use double quotes to ensure proper expansion.
Let's update line 478 as follows:
- su postgres -c "env PATH='$PATH' psql -c 'CREATE DATABASE ${APPSMITH_PG_DATABASE}'"
+ su postgres -c "env PATH='$PATH' psql -c \"CREATE DATABASE ${APPSMITH_PG_DATABASE}\""This change ensures that our database name is correctly interpreted. Keep up the great work, and always remember to double-check your quotation marks!
📝 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.
| create_appsmith_pg_db() { | |
| POSTGRES_DB_PATH=$1 | |
| # Start the postgres , wait for it to be ready and create a appsmith db | |
| su postgres -c "env PATH='$PATH' pg_ctl -D $POSTGRES_DB_PATH -l $POSTGRES_DB_PATH/logfile start" | |
| echo "Waiting for Postgres to start" | |
| until su postgres -c "env PATH='$PATH' pg_isready -d postgres"; do | |
| tlog "Waiting for Postgres to be ready..." | |
| sleep 1 | |
| done | |
| # Check if the appsmith DB is present | |
| DB_EXISTS=$(su postgres -c "env PATH='$PATH' psql -tAc \"SELECT 1 FROM pg_database WHERE datname='${APPSMITH_PG_DATABASE}'\"") | |
| if [[ "$DB_EXISTS" != "1" ]]; then | |
| su postgres -c "env PATH='$PATH' psql -c 'CREATE DATABASE ${APPSMITH_PG_DATABASE}'" | |
| else | |
| echo "Database ${APPSMITH_PG_DATABASE} already exists." | |
| fi | |
| su postgres -c "env PATH='$PATH' pg_ctl -D $POSTGRES_DB_PATH stop" | |
| } | |
| create_appsmith_pg_db() { | |
| POSTGRES_DB_PATH=$1 | |
| # Start the postgres , wait for it to be ready and create a appsmith db | |
| su postgres -c "env PATH='$PATH' pg_ctl -D $POSTGRES_DB_PATH -l $POSTGRES_DB_PATH/logfile start" | |
| echo "Waiting for Postgres to start" | |
| until su postgres -c "env PATH='$PATH' pg_isready -d postgres"; do | |
| tlog "Waiting for Postgres to be ready..." | |
| sleep 1 | |
| done | |
| # Check if the appsmith DB is present | |
| DB_EXISTS=$(su postgres -c "env PATH='$PATH' psql -tAc \"SELECT 1 FROM pg_database WHERE datname='${APPSMITH_PG_DATABASE}'\"") | |
| if [[ "$DB_EXISTS" != "1" ]]; then | |
| su postgres -c "env PATH='$PATH' psql -c \"CREATE DATABASE ${APPSMITH_PG_DATABASE}\"" | |
| else | |
| echo "Database ${APPSMITH_PG_DATABASE} already exists." | |
| fi | |
| su postgres -c "env PATH='$PATH' pg_ctl -D $POSTGRES_DB_PATH stop" | |
| } |
There was a problem hiding this comment.
@pratapaprasanna, excellent! Applying the same correction here will ensure our script functions flawlessly. Consistency is crucial, and it's great to see you addressing this. Keep up the fantastic work!
🐰🎉
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
Description
With the migration to postgres as Appsmith's persistent database we wanted to have central DB to be consumed by internal services. Currently all the services try to create the same DB and may end up in a race condition, so we are moving this task to
entrypoint.sh.Steps:
appsmithdb with postgres user/test Sanity
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11157110599
Commit: c1ef99a
Cypress dashboard.
Tags:
@tag.SanitySpec:
Thu, 03 Oct 2024 07:25:01 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes