-
Notifications
You must be signed in to change notification settings - Fork 0
Update setup script and add sample configuration file #30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds a new production configuration file and simplifies the environment setup script by removing SQLite-related exports and setup logic while updating the configuration download URL to point to the folder-hashing-api repository. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/env-setup.sh (1)
158-160: Remove lines 158-160: orphaned code referencing undefined$DB_PATH_BASEvariable.The variable
$DB_PATH_BASEis not defined anywhere in the script. These three lines are remnants from the removed SQLite setup and will cause the script to fail at runtime. Since other SQLite-related variables were successfully removed, these lines should be deleted as well:chown -R $RUNTIME_USER:$RUNTIME_USER "$DB_PATH_BASE" find "$DB_PATH_BASE" -type d -exec chmod 0750 "{}" \; find "$DB_PATH_BASE" -type f -exec chmod 0640 "{}" \;
🧹 Nitpick comments (1)
scripts/env-setup.sh (1)
17-17: Update help text to reflect removed SQLite setup.Line 17 in the help text mentions "skip SQLite setup" for the
--forceflag, but per the PR changes, SQLite setup has been completely removed. Update the help text to remove this outdated reference.- echo " -f | --force Run without interactive prompts (skip questions, skip SQLite setup, do not overwrite config)" + echo " -f | --force Run without interactive prompts (skip questions, do not overwrite config)"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
configuration/app-config-prod.json(1 hunks)scripts/env-setup.sh(1 hunks)
🔇 Additional comments (3)
configuration/app-config-prod.json (2)
6-7: Verify port number types and OltpExporter address configuration.Lines 6-7 specify port numbers as strings rather than numbers (JSON numbers would be more conventional). If the application expects number types, consider removing quotes. Additionally, line 11 specifies
"OltpExporter": "0.0.0.0:4317"— the0.0.0.0address is typically used for server binds (listening), not for outbound/client connections. Ensure this is the intended configuration; considerlocalhost:4317or a specific service hostname for external telemetry connections.
10-10: Telemetry enabled by default in production.Line 10 sets telemetry to enabled by default in the production configuration. Verify this aligns with your security and privacy policies. Consider whether telemetry should be opt-in rather than enabled by default.
scripts/env-setup.sh (1)
46-46: Configuration download URL updated correctly.The updated URL now correctly points to the folder-hashing-api repository's production configuration file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/env-setup.sh (1)
17-17: Update help text to reflect removal of SQLite setup logic.The help text mentions "skip SQLite setup" but per the PR changes, SQLite database setup logic has been removed. Update the help text to accurately reflect the current functionality.
Apply this diff to clarify the
-f|--forceflag's behavior:- echo " -f | --force Run without interactive prompts (skip questions, skip SQLite setup, do not overwrite config)" + echo " -f | --force Run without interactive prompts (skip questions, do not overwrite config)"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/env-setup.sh(2 hunks)
🔇 Additional comments (1)
scripts/env-setup.sh (1)
101-102: Good: Added executable permission for startup script.The
chmod +xensures the copied startup script is executable, which is necessary for systemd to run it properly. The error handling is consistent with the rest of the script.
| export SQLITE_DB_NAME=base.sqlite | ||
| export TARGET_SQLITE_DB_NAME=db.sqlite | ||
| export CONF_DOWNLOAD_URL="https://raw.githubusercontent.com/scanoss/hfh/refs/heads/main/config/app-config-prod.json" | ||
| export CONF_DOWNLOAD_URL="https://raw.githubusercontent.com/scanoss/folder-hashing-api/refs/heads/main/configuration/app-config-prod.json" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify the new configuration file is accessible at the updated URL.
The CONF_DOWNLOAD_URL now references a new production configuration file. Ensure that configuration/app-config-prod.json exists in the repository and is accessible via the GitHub raw content URL, and that the change aligns with the new configuration file added in this PR.
🏁 Script executed:
#!/bin/bash
# Description: Verify the configuration file exists and is accessible
# Check if the file exists in the repository
fd -t f "app-config-prod.json" --max-one-result
# Verify the GitHub raw URL would be accessible
curl -sI "https://raw.githubusercontent.com/scanoss/folder-hashing-api/refs/heads/main/configuration/app-config-prod.json" | head -n 1Length of output: 245
🏁 Script executed:
#!/bin/bash
# Search for configuration files with corrected fd syntax
echo "=== Searching for configuration files ==="
fd -t f "\.json$" | grep -i config | head -20
echo ""
echo "=== Checking specific paths ==="
find . -type f -name "*app-config*" 2>/dev/null
find . -type d -name "configuration" 2>/dev/null
echo ""
echo "=== Checking for any JSON files in configuration directory if it exists ==="
if [ -d "configuration" ]; then
ls -la configuration/
else
echo "configuration/ directory does not exist"
fiLength of output: 437
🏁 Script executed:
#!/bin/bash
# Check the help text at line 17 and search for SQLite references
echo "=== Lines around line 17 (help text) ==="
sed -n '15,20p' scripts/env-setup.sh
echo ""
echo "=== Searching for SQLite references in the script ==="
grep -n -i "sqlite" scripts/env-setup.sh
echo ""
echo "=== Checking line 102 area for chmod +x ==="
sed -n '100,105p' scripts/env-setup.shLength of output: 960
Update help text to remove outdated SQLite reference.
The help text at line 17 mentions "skip SQLite setup," but the script no longer contains SQLite setup logic. Update the help text to remove this reference and accurately describe the -f | --force flag's actual behavior:
-f | --force Run without interactive prompts (skip questions, do not overwrite config)
The configuration file verification passed—configuration/app-config-prod.json exists in the repository with the correct path, and the CONF_DOWNLOAD_URL is properly configured.
🤖 Prompt for AI Agents
In scripts/env-setup.sh around lines 17 and 46, the help text still mentions
"skip SQLite setup" which is outdated; update the help/usage block to remove any
reference to SQLite and change the -f | --force flag description to: "Run
without interactive prompts (skip questions, do not overwrite config)". Leave
the CONF_DOWNLOAD_URL at line 46 unchanged since the configuration file path is
correct.
Summary by CodeRabbit