diff options
Diffstat (limited to 'llamachat/providers.py')
| -rw-r--r-- | llamachat/providers.py | 75 |
1 files changed, 43 insertions, 32 deletions
diff --git a/llamachat/providers.py b/llamachat/providers.py index 4c3e774..ce2d01d 100644 --- a/llamachat/providers.py +++ b/llamachat/providers.py @@ -17,6 +17,7 @@ import math import os import subprocess import sys +import threading from dataclasses import dataclass, field # The local llama.cpp router. Its models are shown and stored without a @@ -81,25 +82,35 @@ def _not_a_table(name: str, entry) -> str: return f"a provider must be a [providers.{name}] table" -def _skipped(name: str, reason: str) -> None: +def _skipped(name: str, reason: str, warnings: list[str] | None = None) -> None: """Say that a provider was dropped, so the loss is not silent. A skipped provider is invisible: its models are simply absent from the picker, which looks identical to the provider being down. One stderr line gives a user running from a terminal something to act on. + When `warnings` is provided, the message is also collected there so the + UI can surface it in the status bar — a desktop launch has no terminal, + so stderr alone is invisible to the user. + ponytail: stderr only, so a launch from a .desktop file still says nothing. Surfacing this in the window is Task 12's job; there is no window to put it in yet. """ - print(f"llamachat: ignoring provider {name!r}: {reason}", file=sys.stderr) + msg = f"ignoring provider {name!r}: {reason}" + print(f"llamachat: {msg}", file=sys.stderr) + if warnings is not None: + warnings.append(msg) -def parse(values: dict) -> dict[str, Provider]: +def parse(values: dict, warnings: list[str] | None = None) -> dict[str, Provider]: """Build the provider table from already-loaded config values. Takes the raw dict rather than a path so config.py owns file reading and this stays testable without touching disk. + + When `warnings` is provided, skipped-provider messages are collected + there so the UI can surface them in the status bar. """ raw = values.get("providers") # `providers = "oops"` is valid TOML, and coercing it raises: ValueError @@ -128,7 +139,7 @@ def parse(values: dict) -> dict[str, Provider]: # something matters most, not least. The app comes up working, local # answers, and an api_key or ctx_size they set is simply gone, with a # healthy-looking window as the only feedback. - _skipped(LOCAL, _not_a_table(LOCAL, raw_local)) + _skipped(LOCAL, _not_a_table(LOCAL, raw_local), warnings) local = raw_local if isinstance(raw_local, dict) else {} if bare and not local.get("base_url"): table[LOCAL] = {**local, "base_url": bare} @@ -140,20 +151,20 @@ def parse(values: dict) -> dict[str, Provider]: # Most likely [[providers.x]] written for [providers.x], which # TOML reads as a list of tables. Skipping keeps every other # provider working, including local. - _skipped(name, _not_a_table(name, entry)) + _skipped(name, _not_a_table(name, entry), warnings) continue if not name or ":" in name: # The id scheme splits on the first colon, so a name containing # one builds ids that split back to something else entirely, and # an empty name builds ":model". Both route to local under a # nonsense name, silently. Skipping is the only honest option. - _skipped(name, "a provider name cannot be empty or contain ':'") + _skipped(name, "a provider name cannot be empty or contain ':'", warnings) continue base_url = str(entry.get("base_url") or "").rstrip("/") if not base_url: # ponytail: a provider with no URL is misconfigured, not a # partial one. Skipping beats inventing a default endpoint. - _skipped(name, "it has no base_url") + _skipped(name, "it has no base_url", warnings) continue # filter = "qwen" is an easy TOML slip for filter = ["qwen"], and # iterating the string would turn it into four single-character @@ -242,16 +253,15 @@ class KeyResolver: def __init__(self, runner=_run_pass): self._runner = runner - # Not locked, and that is only safe while every caller resolves on the - # GUI thread, where the Qt event loop serializes them. Moving a - # resolve onto a worker thread, or threading the model fan-out, breaks - # it: two threads can miss the cache together, spawn two `pass` - # processes and raise two pinentry prompts for one token. Whoever does - # that adds a threading.Lock here and holds it across the whole - # resolve() body, not just the dict access, since guarding only the - # dict leaves both threads outside the lock during the subprocess, - # which is the double-prompt window itself. + # Locked: resolve() now runs on worker threads (StreamWorker.run and + # TitleWorker.run can overlap), so the lock serializes the entire + # resolve body, including the subprocess call. Guarding only the dict + # would leave both threads outside the lock during `pass show`, which + # is the double-prompt window: two threads miss the cache together, + # spawn two `pass` processes and raise two pinentry prompts for one + # hardware token. self._cache: dict[tuple[str, str], str] = {} + self._lock = threading.Lock() def resolve(self, provider: Provider) -> str: """The bearer token for this provider, or '' when it needs none.""" @@ -263,22 +273,23 @@ class KeyResolver: # the old key after the user edits the entry to fix it, so the fix # would look like it did nothing until a restart. cache_key = (provider.name, spec) - if cache_key in self._cache: - return self._cache[cache_key] - - if spec.startswith("env:"): - value = os.environ.get(spec[4:], "") - if not value: - raise KeyResolutionError( - f"{provider.name}: environment variable {spec[4:]} is not set" - ) - elif spec.startswith("pass:"): - value = self._from_pass(provider, spec[5:]) - else: - value = spec - - self._cache[cache_key] = value - return value + with self._lock: + if cache_key in self._cache: + return self._cache[cache_key] + + if spec.startswith("env:"): + value = os.environ.get(spec[4:], "") + if not value: + raise KeyResolutionError( + f"{provider.name}: environment variable {spec[4:]} is not set" + ) + elif spec.startswith("pass:"): + value = self._from_pass(provider, spec[5:]) + else: + value = spec + + self._cache[cache_key] = value + return value def _from_pass(self, provider: Provider, entry: str) -> str: try: |