1899 Updated db_upgrade scripts for admin-services#1387
1899 Updated db_upgrade scripts for admin-services#1387abhishek8shankar wants to merge 2 commits into
Conversation
Signed-off-by: abhishek8shankar <abhishek.shankarcs@gmail.com>
|
Warning Review limit reached
Next review available in: 10 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughHard-coded database names (mosip_hotlist, mosip_master) and role names (hotlistuser, masteruser) in SQL upgrade/rollback scripts are replaced with psql bind variables (:mosipdbname, :dbuname). Corresponding upgrade.properties files add a DB_UNAME entry, and upgrade.sh scripts pass these values as psql -v parameters. Changesmosip_hotlist DB parameterization
mosip_master DB parameterization
Estimated code review effort: 2 (Simple) | ~15 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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.
🧹 Nitpick comments (4)
db_upgrade_scripts/mosip_hotlist/upgrade.sh (2)
24-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnnecessary
-vvariables on the terminate-connections call.
mosipdbname/dbunameare passed but the inline-cquery only references$MOSIP_DB_NAME(shell var), not:mosipdbname. Harmless but dead parameters.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@db_upgrade_scripts/mosip_hotlist/upgrade.sh` at line 24, The terminate-connections psql invocation in upgrade.sh passes unused -v variables, since the SQL in the -c block references the shell variable MOSIP_DB_NAME directly instead of the psql variables mosipdbname/dbuname. Remove the dead variable bindings from the psql command (or switch the query to use the named psql variables consistently) so the call only keeps the parameters actually used.
24-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnquoted shell variables in psql invocations (SC2086).
Static analysis flags the newly added
mosipdbname=$MOSIP_DB_NAME/dbuname=$DB_UNAMEargs (and pre-existing ones) as unquoted, risking word-splitting/globbing if values contain spaces/special chars.Also applies to: 33-33, 43-43
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@db_upgrade_scripts/mosip_hotlist/upgrade.sh` at line 24, The psql call in upgrade.sh is passing shell-expanded values such as SU_USER_PWD, MOSIP_DB_NAME, DB_UNAME, SU_USER, DB_SERVERIP, DB_PORT, and DEFAULT_DB_NAME without quotes, which can trigger word-splitting or globbing; update the command in the upgrade flow so each variable expansion is quoted or otherwise safely passed when invoking psql. Apply the same fix consistently in the other affected psql invocations in this script, including the ones used alongside the CONN assignment and any related database checks.Source: Linters/SAST tools
db_upgrade_scripts/mosip_master/upgrade.sh (2)
24-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnused
CONNand no-opexitinside command substitution.
CONNis assigned but never read afterward, andexitruns inside the$(...)subshell so it doesn't actually stop the script if terminating connections fails — execution proceeds to the upgrade/rollback step regardless.♻️ Optional cleanup
-CONN=$(PGPASSWORD=$SU_USER_PWD psql -v ON_ERROR_STOP=1 -v mosipdbname=$MOSIP_DB_NAME -v dbuname=$DB_UNAME --username=$SU_USER --host=$DB_SERVERIP --port=$DB_PORT --dbname=$DEFAULT_DB_NAME -t -c "SELECT count(pg_terminate_backend(pg_stat_activity.pid)) FROM pg_stat_activity WHERE datname = '$MOSIP_DB_NAME' AND pid <> pg_backend_pid()";exit;) +PGPASSWORD=$SU_USER_PWD psql -v ON_ERROR_STOP=1 -v mosipdbname=$MOSIP_DB_NAME -v dbuname=$DB_UNAME --username=$SU_USER --host=$DB_SERVERIP --port=$DB_PORT --dbname=$DEFAULT_DB_NAME -t -c "SELECT count(pg_terminate_backend(pg_stat_activity.pid)) FROM pg_stat_activity WHERE datname = '$MOSIP_DB_NAME' AND pid <> pg_backend_pid()"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@db_upgrade_scripts/mosip_master/upgrade.sh` at line 24, The connection-termination step in upgrade.sh is assigning the result to CONN even though it is never used, and the trailing exit inside the command substitution does not stop the main script. Update the psql/pg_terminate_backend logic in the CONN assignment so failures are handled in the parent shell, and ensure the upgrade flow halts before the rollback/upgrade step if terminating active sessions does not succeed.Source: Linters/SAST tools
24-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueQuote variable expansions in
-vargs (shellcheck SC2086).
$MOSIP_DB_NAME,$DB_UNAME, etc. are unquoted in thepsql -vinvocations, risking word-splitting/globbing if a value ever contains spaces/special chars.Also applies to: 33-33, 43-43
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@db_upgrade_scripts/mosip_master/upgrade.sh` at line 24, The psql invocation in the upgrade script passes shell variables into -v without quoting, which can trigger word splitting or globbing if values contain spaces or special characters. Update the psql command(s) that set mosipdbname, dbuname, and any similar variables to use properly quoted variable expansions, and apply the same fix to the other affected psql calls in the script so the values are passed safely and consistently.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@db_upgrade_scripts/mosip_hotlist/upgrade.sh`:
- Line 24: The terminate-connections psql invocation in upgrade.sh passes unused
-v variables, since the SQL in the -c block references the shell variable
MOSIP_DB_NAME directly instead of the psql variables mosipdbname/dbuname. Remove
the dead variable bindings from the psql command (or switch the query to use the
named psql variables consistently) so the call only keeps the parameters
actually used.
- Line 24: The psql call in upgrade.sh is passing shell-expanded values such as
SU_USER_PWD, MOSIP_DB_NAME, DB_UNAME, SU_USER, DB_SERVERIP, DB_PORT, and
DEFAULT_DB_NAME without quotes, which can trigger word-splitting or globbing;
update the command in the upgrade flow so each variable expansion is quoted or
otherwise safely passed when invoking psql. Apply the same fix consistently in
the other affected psql invocations in this script, including the ones used
alongside the CONN assignment and any related database checks.
In `@db_upgrade_scripts/mosip_master/upgrade.sh`:
- Line 24: The connection-termination step in upgrade.sh is assigning the result
to CONN even though it is never used, and the trailing exit inside the command
substitution does not stop the main script. Update the psql/pg_terminate_backend
logic in the CONN assignment so failures are handled in the parent shell, and
ensure the upgrade flow halts before the rollback/upgrade step if terminating
active sessions does not succeed.
- Line 24: The psql invocation in the upgrade script passes shell variables into
-v without quoting, which can trigger word splitting or globbing if values
contain spaces or special characters. Update the psql command(s) that set
mosipdbname, dbuname, and any similar variables to use properly quoted variable
expansions, and apply the same fix to the other affected psql calls in the
script so the values are passed safely and consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 29d9586f-26f6-4fb1-bbe5-4ef1249d357d
📒 Files selected for processing (10)
db_upgrade_scripts/mosip_hotlist/sql/1.1.5.5_to_1.2.0.1-B1_rollback.sqldb_upgrade_scripts/mosip_hotlist/sql/1.1.5.5_to_1.2.0.1-B1_upgrade.sqldb_upgrade_scripts/mosip_hotlist/upgrade.propertiesdb_upgrade_scripts/mosip_hotlist/upgrade.shdb_upgrade_scripts/mosip_master/sql/1.1.5.5_to_1.2.0.1-B1_rollback.sqldb_upgrade_scripts/mosip_master/sql/1.1.5.5_to_1.2.0.1-B1_upgrade.sqldb_upgrade_scripts/mosip_master/sql/1.2.1.4_to_1.3.0_rollback.sqldb_upgrade_scripts/mosip_master/sql/1.2.1.4_to_1.3.0_upgrade.sqldb_upgrade_scripts/mosip_master/upgrade.propertiesdb_upgrade_scripts/mosip_master/upgrade.sh
Signed-off-by: abhishek8shankar <abhishek.shankarcs@gmail.com>
1 participant
Summary by CodeRabbit