-
Notifications
You must be signed in to change notification settings - Fork 0
fix(routing): implement pure subdomain routing for LiteLLM, Langfuse, and Llama #342
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3546,21 +3546,15 @@ def resolve_external_urls(request: Request) -> tuple[str, str, str]: | |
| is_valid_external = external_host == domain or external_host.endswith("." + domain) | ||
| is_valid_base = request.base_url.hostname == domain or (request.base_url.hostname or "").endswith("." + domain) | ||
|
|
||
| if is_valid_external: | ||
| # Centralized base URL path under subdomain/reverse proxy | ||
| if is_valid_external or is_valid_base: | ||
| host_val = external_host if is_valid_external else (request.base_url.hostname or "vendeuvre.lan") | ||
| host_base = host_val.replace("dashboard.", "") | ||
| if host_base.startswith("litellm.") or host_base.startswith("langfuse.") or host_base.startswith("llama."): | ||
| host_base = re.sub(r"^(litellm|langfuse|llama)\.", "", host_base) | ||
| return ( | ||
| f"{external_scheme}://{external_netloc}/llm-routing/langfuse", | ||
| f"{external_scheme}://{external_netloc}/llm-routing/litellm/ui", | ||
| f"{external_scheme}://{external_netloc}/llm-routing/llama/" | ||
| ) | ||
| elif is_valid_base: | ||
| parsed_netloc = urlparse(f"{external_scheme}://{request.url.netloc}") | ||
| netloc = request.url.netloc if parsed_netloc.hostname else "localhost" | ||
| base = f"{external_scheme}://{netloc}" | ||
| return ( | ||
| f"{base}/llm-routing/langfuse", | ||
| f"{base}/llm-routing/litellm/ui", | ||
| f"{base}/llm-routing/llama/" | ||
| f"{external_scheme}://langfuse.{host_base}", | ||
| f"{external_scheme}://litellm.{host_base}/ui/", | ||
| f"{external_scheme}://llama.{host_base}/" | ||
|
Comment on lines
+3549
to
+3557
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Update the resolver tests to the new subdomain contract.
🤖 Prompt for AI Agents
Comment on lines
3554
to
+3557
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Preserve an explicit public port when constructing subdomain URLs.
Carry the explicit port from the configured/request netloc into all three returned URLs. 🤖 Prompt for AI Agents |
||
| ) | ||
| else: | ||
| # Local development fallback: derive schemes, ports, and paths dynamically from configuration constants | ||
|
|
||
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.
suggestion (bug_risk): Hardcoded "vendeuvre.lan" fallback may be surprising and environment-specific.
This hardcoded fallback in
host_valrisks misrouting in non-vendeuvre environments ifrequest.base_url.hostnameis unset or misconfigured. Consider deriving the fallback from configuration (e.g.,ROUTING_DOMAIN), or failing fast/logging when neitherexternal_hostnor a valid base hostname is available, instead of silently defaulting to a specific LAN domain.Suggested implementation:
If your routing domain is configurable via a dedicated setting (e.g.
ROUTING_DOMAIN), you may want to:domainfrom that configuration, if it is not already.domainis determined to surface misconfigurations (e.g. whenrequest.base_url.hostnameis unexpectedlyNoneand the code falls back todomain).