move app. #1494
move app. #1494
Conversation
📝 WalkthroughWalkthroughDatabase schema creation was moved out of shell scripts into a centralized Changes
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
install/production-filesystem/entrypoint.d/25-first-run-db.sh (1)
61-85:⚠️ Potential issue | 🔴 Critical
set -eu(line 11) makes the error-handling block on lines 63–85 unreachable.Because
set -eis active, ifsqlite3on line 61 exits non-zero, the shell terminates immediately —database_creation_status=$?on line 63 is never reached, and the user never sees the helpful CRITICAL error message.You need to suppress
set -efor thesqlite3call so the exit code can be captured and the custom error block runs.Proposed fix
-sqlite3 "${NETALERTX_DB_FILE}" < "${NETALERTX_SERVER}/db/schema/app.sql" - -database_creation_status=$? - -if [ $database_creation_status -ne 0 ]; then +if ! sqlite3 "${NETALERTX_DB_FILE}" < "${NETALERTX_SERVER}/db/schema/app.sql"; then RED=$(printf '\033[1;31m') RESET=$(printf '\033[0m')
🤖 Fix all issues with AI agents
In `@install/production-filesystem/entrypoint.d/25-first-run-db.sh`:
- Around line 60-61: The comment above the sqlite3 invocation is stale; update
the comment near the sqlite3 "${NETALERTX_DB_FILE}" <
"${NETALERTX_SERVER}/db/schema/app.sql" line in the 25-first-run-db.sh script to
describe that the schema is being loaded from the app.sql file (via file
redirection) rather than from a heredoc, referencing the NETALERTX_DB_FILE
target and NETALERTX_SERVER/db/schema/app.sql source; replace the old "Write all
text to db file until we see 'end-of-database-schema'" text with a concise note
like "Load database schema from app.sql into NETALERTX_DB_FILE" (or similar) so
the comment matches the current file-based approach.
In `@scripts/db_cleanup/regenerate-database.sh`:
- Line 20: The sqlite3 schema import command (sqlite3 "${NETALERTX_DB_FILE}" <
"${SCHEMA_FILE}") lacks error handling; update regenerate-database.sh to detect
failures from that command and fail fast with a clear error message: either
enable strict error handling (e.g., set -e at script top) or check the exit
status ($?) immediately after the sqlite3 invocation and call echo/process exit
with a non-zero code if it failed, including the NETALERTX_DB_FILE and
SCHEMA_FILE in the message to aid debugging.
- Line 7: The script currently removes only the main DB file
(${NETALERTX_DB_FILE}) and fails to remove SQLite WAL/SHM sidecar files, which
can leave stale state; update the cleanup to also remove the two sidecar files
for the same base name (e.g., "${NETALERTX_DB_FILE}-wal" and
"${NETALERTX_DB_FILE}-shm" or use a glob like "${NETALERTX_DB_FILE}"*), ensuring
you check for their existence before removal so the regenerate-database.sh
script deletes the WAL and SHM files along with the main DB to avoid corruption;
locate the removal call around rm "${NETALERTX_DB_FILE}" and extend it to
include the sidecar filenames.
In `@server/db/schema/app.sql`:
- Around line 296-306: The inline comment for the literal 'DEVICES' is
incorrect: in the INSERT into the audit table the literal maps to the
ObjectPlugin column (not ObjectForeignKey). Update the comments in all three
triggers (trg_insert_devices, trg_update_devices, trg_delete_devices) so the
`'DEVICES'` entry is annotated as "-- ObjectPlugin" (the ObjectForeignKey
comment should remain on the preceding NEW.devGUID / OLD.devGUID value).
- Around line 206-228: The view-column comment blocks for Events_Devices,
LatestEventsPerMAC and Sessions_Devices are stale and missing the Devices
columns devParentRelType and devFQDN; update each trailing /* ... */ comment to
include devParentRelType and devFQDN (matching the Devices table's column names)
so the documented column lists for the views reflect the current schema (refer
to the Devices table and the view names Events_Devices, LatestEventsPerMAC,
Sessions_Devices to locate the comments).
🧹 Nitpick comments (1)
server/db/schema/app.sql (1)
1-2: InconsistentIF NOT EXISTSusage across table definitions.Some tables use
CREATE TABLE IF NOT EXISTS(Online_History, Settings, Parameters, AppEvents, Notifications) while others use bareCREATE TABLE(Events, Sessions, Devices, Plugins_Objects, Plugins_Events, Plugins_History, Plugins_Language_Strings, CurrentScan). Since this file is intended for fresh database creation, this may be intentional, but it means re-running this file against an existing database will fail on the tables withoutIF NOT EXISTS. Consider making the usage consistent for robustness.Also applies to: 13-46, 47-57, 58-61, 62-84, 85-106, 107-136, 137-152, 153-174, 175-187
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
thanks @adamoutler 🙏 |
2 participants
📌 Description
Add separate app.sql. Re-reference from scripts.
🔍 Related Issues
📋 Type of Change
Please check the relevant option(s):
📷 Screenshots or Logs (if applicable)
🧪 Testing Steps
Full suite
✅ Checklist
🙋 Additional Notes
Summary by CodeRabbit
Refactor
Chores