feat: Introduce backup and restore for postgres #37326
Conversation
(cherry picked from commit 588e792)
WalkthroughThe changes in this pull request enhance backup functionality by integrating PostgreSQL support alongside existing MongoDB capabilities. Modifications include the addition of a new Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Warning Rate limit exceeded@sharat87 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 20 minutes and 20 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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
abhvsn
changed the title
abhvsn
added
Enhancement
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/11784388675. |
1 similar comment
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/11784388675. |
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/11784652237. |
|
Deploy-Preview-URL: https://ce-37326.dp.appsmith.com |
github-actions
bot
added
DB Infrastructure Pod
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Outside diff range and nitpick comments (7)
deploy/docker/fs/opt/appsmith/utils/bin/export_db.js (1)
Line range hint
43-65: Improve error handling and application state managementThe error handling could be more specific and ensure proper application restart in all scenarios.
+ const DatabaseExportError = class extends Error { + constructor(message, cause) { + super(message); + this.cause = cause; + this.name = 'DatabaseExportError'; + } + }; + try { check_supervisord_status_cmd = '/usr/bin/supervisorctl >/dev/null 2>&1 '; shell.exec(check_supervisord_status_cmd, function (code) { if (code > 0) { - shell.echo('application is not running, starting supervisord'); + throw new DatabaseExportError('Supervisord is not running'); shell.exec('/usr/bin/supervisord'); } });deploy/docker/fs/opt/appsmith/utils/bin/utils.js (1)
Add PostgreSQL support and remove MongoDB dependencies in utils.js
The
preprocessMongoDBURIfunction is still present and PostgreSQL support is not implemented.
deploy/docker/fs/opt/appsmith/utils/bin/utils.js(Line 125)🔗 Analysis chain
Line range hint
1-201: Verify PostgreSQL support in utility functionsSeveral utility functions in this file were originally designed for MongoDB. Let's verify they work correctly with PostgreSQL.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for PostgreSQL-specific handling in the codebase rg -l 'postgres|postgresql' --type js # Check for any potential MongoDB-specific operations that might need PostgreSQL equivalents ast-grep --pattern 'MongoClient|mongoose|mongodb'Length of output: 1305
Script:
#!/bin/bash # Verify PostgreSQL support within utils.js ast-grep --pattern 'PostgreSQL|pg' deploy/docker/fs/opt/appsmith/utils/bin/utils.js # Check if MongoDB functions are still being used or referenced in utils.js ast-grep --pattern 'preprocessMongoDBURI' deploy/docker/fs/opt/appsmith/utils/bin/utils.jsLength of output: 279
deploy/docker/fs/opt/appsmith/utils/bin/backup.test.js (1)
59-67: Add test cases for edge cases in PostgreSQL connection stringWhile the basic test case looks good, consider adding tests for:
- Invalid connection strings
- Connection strings with special characters
- Different PostgreSQL connection string formats
deploy/docker/fs/opt/appsmith/utils/bin/import_db.js (2)
38-46: Avoid redundantutils.getDburl()callsPass
dbUrlas a parameter toget_table_or_collection_lento prevent multiple calls.Apply this diff:
- function get_table_or_collection_len() { + function get_table_or_collection_len(dbUrl) {Update the call at line 63:
- const collectionsLen = get_table_or_collection_len(); + const collectionsLen = get_table_or_collection_len(dbUrl);
12-16: Handle unsupported database typesAdd an
elseclause to handle cases where the database type is neither MongoDB nor PostgreSQL.Apply this diff:
} else if (dbUrl.startsWith('postgresql')) { restore_postgres_db(dbUrl); + } else { + console.error(`Unsupported database type: ${dbUrl}`); + process.exit(1); }deploy/docker/fs/opt/appsmith/utils/bin/backup.js (1)
128-133: Store database URL in a variable to avoid multiple function callsCurrently,
utils.getDburl()is called multiple times. Consider storing the result in a variable for efficiency.Apply this diff to implement the change:
+ const dbUrl = utils.getDburl(); - if (utils.getDburl().startsWith('mongodb')) { + if (dbUrl.startsWith('mongodb')) { - await executeMongoDumpCMD(destFolder, utils.getDburl()) + await executeMongoDumpCMD(destFolder, dbUrl) - } else if (utils.getDburl().startsWith('postgresql')) { + } else if (dbUrl.startsWith('postgresql')) { - await executePostgresDumpCMD(destFolder, utils.getDburl()); + await executePostgresDumpCMD(destFolder, dbUrl); }deploy/docker/fs/opt/appsmith/utils/bin/restore.js (1)
64-65: Use consistent methods for database type checksFor consistency, use
dbUrl.startsWith('postgresql')instead ofdbUrl.includes('postgresql')when checking for PostgreSQL URLs.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (7)
app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/CommonDBConfig.java(1 hunks)deploy/docker/fs/opt/appsmith/utils/bin/backup.js(4 hunks)deploy/docker/fs/opt/appsmith/utils/bin/backup.test.js(2 hunks)deploy/docker/fs/opt/appsmith/utils/bin/export_db.js(1 hunks)deploy/docker/fs/opt/appsmith/utils/bin/import_db.js(3 hunks)deploy/docker/fs/opt/appsmith/utils/bin/restore.js(1 hunks)deploy/docker/fs/opt/appsmith/utils/bin/utils.js(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/CommonDBConfig.java
🔇 Additional comments (4)
deploy/docker/fs/opt/appsmith/utils/bin/utils.js (1)
198-199: LGTM: Module exports are properly updated
The new functions are correctly exported.
deploy/docker/fs/opt/appsmith/utils/bin/import_db.js (2)
26-26: Use the correct database name in pg_restore command
The database name is hardcoded as appsmith. Ensure this matches the database in dbUrl or extract the database name dynamically.
Consider updating the command:
- const cmd = `pg_restore -U postgres -d appsmith --verbose --clean ${Constants.RESTORE_PATH}/${Constants.DUMP_FILE_NAME}`;
+ const dbName = utils.getDbNameFromUrl(dbUrl);
+ const cmd = `pg_restore -U postgres -d ${dbName} --verbose --clean ${Constants.RESTORE_PATH}/${Constants.DUMP_FILE_NAME}`;Ensure you have a utility function getDbNameFromUrl to extract the database name.
25-28: 🛠️ Refactor suggestion
Declare restore_postgres_db with const and pass dbUrl as a parameter
For consistency and proper scope, declare the function with const and pass dbUrl.
Apply this diff:
- restore_postgres_db = (dbUrl) => {
+ const restore_postgres_db = (dbUrl) => {Likely invalid or redundant comment.
deploy/docker/fs/opt/appsmith/utils/bin/restore.js (1)
86-87: Verify getDatabaseNameFromDBURI supports PostgreSQL URIs
Confirm that utils.getDatabaseNameFromDBURI correctly parses database names from PostgreSQL URIs, as it now handles both MongoDB and PostgreSQL.
github-actions
bot
added
the
Platform Administration Pod
|
This PR has been closed because of inactivity. |
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/13129876896. |
|
Deploy-Preview-URL: https://ce-37326.dp.appsmith.com |
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/13183201360. |
|
Deploy-Preview-URL: https://ce-37326.dp.appsmith.com |
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/13198341017. |
|
Deploy-Preview-URL: https://ce-37326.dp.appsmith.com |
Description
PR to add backup and restore functionality for Postgres.
Added support for
backupandrestorecommands, but not toimport-dbandexport-db. They are discouraged already anyway, and we can add support to them later if it's really needed. Or actually deprecate them and remove them along with MongoDB itself.Fixes #35369
/test Sanity
🔍 Cypress test results
Warning
Tests have not run on the HEAD 9ff1b9c yet
Fri, 07 Feb 2025 10:51:41 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Release Notes
New Features
PostgresDumpLinkclass for PostgreSQL export functionality.restorePostgresfunction for restoring PostgreSQL databases.Improvements