-
Notifications
You must be signed in to change notification settings - Fork 3
fix(umans): normalize API base and show quota without proxy requests #64
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 |
|---|---|---|
|
|
@@ -93,6 +93,39 @@ def _get_credential_identifier(credential: str) -> str: | |
| return f"{credential[:4]}...{credential[-4:]}" | ||
|
|
||
|
|
||
| def _normalize_umans_api_base(raw: str) -> str: | ||
| """ | ||
| Host root for Umans API paths (/v1/usage, /v1/models). | ||
|
|
||
| Docs often set UMANS_API_BASE to https://api.code.umans.ai/v1 for LiteLLM; | ||
| appending /v1/usage again would hit /v1/v1/usage (404). | ||
| """ | ||
| base = (raw or UMANS_API_BASE_DEFAULT).strip().rstrip("/") | ||
| if base.endswith("/v1"): | ||
| base = base[:-3].rstrip("/") | ||
| return base or UMANS_API_BASE_DEFAULT | ||
|
|
||
|
|
||
| def _resolve_umans_api_key(credential_path: str) -> str: | ||
| """Raw Bearer token for env://umans/N virtual paths.""" | ||
| if not credential_path.startswith("env://"): | ||
| return credential_path | ||
| parts = credential_path[6:].split("/") | ||
| if len(parts) < 2: | ||
| return credential_path | ||
| provider, index_s = parts[0], parts[1] | ||
| if provider != "umans": | ||
| return credential_path | ||
| idx = _safe_int(index_s, 0) | ||
| if idx <= 0: | ||
| key = os.getenv("UMANS_API_KEY", "").strip() | ||
| if key: | ||
| return key | ||
| return credential_path | ||
| key = os.getenv(f"UMANS_API_KEY_{idx}", "").strip() | ||
| return key or credential_path | ||
|
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. SUGGESTION: Silent fallback to the raw A Reply with |
||
|
|
||
|
|
||
| def _parse_iso_to_unix(ts_str: Optional[str]) -> Optional[float]: | ||
| """Parse an ISO 8601 timestamp to a Unix timestamp.""" | ||
| if not ts_str: | ||
|
|
@@ -267,7 +300,9 @@ def set_usage_manager(self, usage_manager: "UsageManager") -> None: | |
| self._usage_manager = usage_manager | ||
|
|
||
| def _resolve_api_base(self) -> str: | ||
| return os.getenv("UMANS_API_BASE", UMANS_API_BASE_DEFAULT).rstrip("/") | ||
| return _normalize_umans_api_base( | ||
| os.getenv("UMANS_API_BASE", UMANS_API_BASE_DEFAULT) | ||
| ) | ||
|
|
||
| async def _fetch_usage_for_credential( | ||
| self, credential_path: str | ||
|
|
@@ -282,9 +317,10 @@ async def _fetch_usage_for_credential( | |
| UmansQuotaSnapshot with status "success" or "error". | ||
| """ | ||
| identifier = _get_credential_identifier(credential_path) | ||
| api_key = _resolve_umans_api_key(credential_path) | ||
| try: | ||
| headers = { | ||
| "Authorization": f"Bearer {credential_path}", | ||
| "Authorization": f"Bearer {api_key}", | ||
| "Accept": "application/json", | ||
| } | ||
| base = self._resolve_api_base() | ||
|
|
||
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: Inconsistent threshold vs. the provider-level branch above.
Line 21 checks
(win.get("total_max") or 0) > 0, but line 26 checkswin.get("limit") is not None. A credential window withlimit == 0(typically meaning "no quota" or "untracked") would still pass and cause the provider to be included in the Web UI quota view with no displayable bar.Consider using the same
> 0semantics for consistency:Reply with
@kilocode-bot fix itto have Kilo Code address this issue.