feat(arr): make the OpenAI endpoint a live setting
The base URL and model move into a cell the backend re-reads per request, so an operator can repoint it without a restart. Migration 0028 adds the two columns; DESIGN.md §15 calls both database rows. Construction never depends on the API key — llama.cpp serves without one.
This commit is contained in:
@@ -0,0 +1,17 @@
|
||||
-- #220. Where the OpenAI-compatible translation backend points, and which
|
||||
-- model it names (DESIGN.md §15). That backend is not "OpenAI" — it is
|
||||
-- anything speaking that request shape, `llama.cpp` and a local gateway
|
||||
-- included — so which endpoint and which model are in use is something the
|
||||
-- operator tries and changes, not a property of the deployment fixed at
|
||||
-- start-up. Both replace bootstrap keys #216 added as an explicit stopgap.
|
||||
--
|
||||
-- Only the API key stays in the environment (§10): it is a secret, and an
|
||||
-- endpoint that needs no key at all is a valid configuration.
|
||||
|
||||
ALTER TABLE subtitle_settings
|
||||
-- NULL means the backend's own default, `https://api.openai.com/v1/`.
|
||||
ADD COLUMN openai_base_url TEXT;
|
||||
|
||||
ALTER TABLE subtitle_settings
|
||||
-- NULL means the backend's own default model.
|
||||
ADD COLUMN openai_model TEXT;
|
||||
@@ -33,6 +33,10 @@ pub mod google;
|
||||
pub mod model;
|
||||
#[cfg(feature = "translate-openai")]
|
||||
pub mod openai;
|
||||
// Not gated: the settings surface must hold and validate this backend's
|
||||
// endpoint whether or not the build compiled the backend in — the
|
||||
// `subtitle_settings` columns exist either way (#220).
|
||||
pub mod openai_endpoint;
|
||||
pub mod opensubtitles;
|
||||
pub mod podnapisi;
|
||||
pub mod srt;
|
||||
@@ -54,6 +58,10 @@ pub use model::{
|
||||
};
|
||||
#[cfg(feature = "translate-openai")]
|
||||
pub use openai::{OpenAi, OpenAiConfig};
|
||||
pub use openai_endpoint::{
|
||||
OpenAiEndpoint, DEFAULT_BASE_URL as OPENAI_DEFAULT_BASE_URL,
|
||||
DEFAULT_MODEL as OPENAI_DEFAULT_MODEL,
|
||||
};
|
||||
pub use opensubtitles::{moviehash, OpenSubtitles, OpenSubtitlesConfig};
|
||||
pub use podnapisi::{Podnapisi, PodnapisiBuilder, DEFAULT_BASE_URL as PODNAPISI_DEFAULT_BASE_URL};
|
||||
pub use srt::Cue;
|
||||
|
||||
@@ -3,10 +3,12 @@
|
||||
//! One HTTP shape — the `/chat/completions` endpoint `OpenAI` defined — reaches
|
||||
//! the largest number of options: `OpenAI` itself, `OpenRouter`, Groq, a
|
||||
//! self-hosted gateway, or a local `llama.cpp` server. The model and base URL
|
||||
//! are constructor arguments rather than fields on [`OpenAiConfig`]: which
|
||||
//! engine runs, and where, is a database setting (DESIGN.md §15, issue #198).
|
||||
//! Only the credential is bootstrap config or environment, per §10 — and even
|
||||
//! that is optional, since a self-hosted gateway may not ask for one.
|
||||
//! live in an [`OpenAiEndpoint`] cell re-read per request rather than on
|
||||
//! [`OpenAiConfig`]: both are database rows the operator edits from
|
||||
//! `/settings`, and an edit reaches this running backend without a restart
|
||||
//! (DESIGN.md §15, issues #198 and #220). Only the credential is bootstrap
|
||||
//! config or environment, per §10 — and even that is optional, since a
|
||||
//! self-hosted gateway or a local `llama.cpp` may not ask for one.
|
||||
//!
|
||||
//! The batch travels as a JSON array of `{number, text}` objects, and the
|
||||
//! reply is asked for in the same shape: JSON round-trips a cue's embedded
|
||||
@@ -20,17 +22,17 @@
|
||||
use std::fmt;
|
||||
use std::time::Duration;
|
||||
|
||||
use reqwest::{Client, StatusCode, Url};
|
||||
use reqwest::{Client, StatusCode};
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
use arr_core::Language;
|
||||
|
||||
use crate::openai_endpoint::OpenAiEndpoint;
|
||||
use crate::translate::{
|
||||
strip_code_fence, system_prompt, Backend, BackendId, Batch, Error, ProbeFuture, Result,
|
||||
TranslateFuture, TranslatedCue,
|
||||
};
|
||||
|
||||
const DEFAULT_BASE_URL: &str = "https://api.openai.com/v1/";
|
||||
const BACKEND_NAME: &str = "openai";
|
||||
const USER_AGENT: &str = "arr v0.1.0";
|
||||
const MAX_ERROR_BODY: usize = 512;
|
||||
@@ -58,35 +60,40 @@ impl fmt::Debug for OpenAiConfig {
|
||||
/// The OpenAI-compatible backend behind the [`Backend`] trait.
|
||||
pub struct OpenAi {
|
||||
http: Client,
|
||||
base_url: Url,
|
||||
endpoint: OpenAiEndpoint,
|
||||
config: OpenAiConfig,
|
||||
model: String,
|
||||
id: BackendId,
|
||||
}
|
||||
|
||||
// Hand-written: the config must never reach a log line.
|
||||
impl fmt::Debug for OpenAi {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
let endpoint = self.endpoint.read();
|
||||
f.debug_struct("OpenAi")
|
||||
.field("base_url", &self.base_url.as_str())
|
||||
.field("model", &self.model)
|
||||
.field("base_url", &endpoint.base_url.as_str())
|
||||
.field("model", &endpoint.model)
|
||||
.finish_non_exhaustive()
|
||||
}
|
||||
}
|
||||
|
||||
impl OpenAi {
|
||||
/// A client against `https://api.openai.com/v1/`.
|
||||
/// A client at the backend's own defaults, ready to be repointed through
|
||||
/// [`Self::endpoint`] once the settings row has been read.
|
||||
///
|
||||
/// Construction never depends on the API key: `llama.cpp` serves without
|
||||
/// authentication, so a base URL and no key is a valid configuration
|
||||
/// (DESIGN.md §15).
|
||||
///
|
||||
/// # Errors
|
||||
///
|
||||
/// Fails if the HTTP client cannot be constructed.
|
||||
pub fn new(model: impl Into<String>, config: OpenAiConfig) -> Result<Self> {
|
||||
Self::with_base_url(model, config, DEFAULT_BASE_URL)
|
||||
pub fn new(config: OpenAiConfig) -> Result<Self> {
|
||||
Self::with_endpoint(config, OpenAiEndpoint::new(None, None)?)
|
||||
}
|
||||
|
||||
/// Point the client at another OpenAI-compatible endpoint — `OpenRouter`,
|
||||
/// Groq, a self-hosted gateway, a local `llama.cpp` server, or `wiremock`
|
||||
/// in tests.
|
||||
/// in tests — with a fixed model.
|
||||
///
|
||||
/// # Errors
|
||||
///
|
||||
@@ -96,24 +103,44 @@ impl OpenAi {
|
||||
config: OpenAiConfig,
|
||||
base_url: &str,
|
||||
) -> Result<Self> {
|
||||
let model = model.into();
|
||||
Self::with_endpoint(
|
||||
config,
|
||||
OpenAiEndpoint::new(Some(base_url), Some(model.as_str()))?,
|
||||
)
|
||||
}
|
||||
|
||||
/// A client re-reading `endpoint` for every request, so the settings API
|
||||
/// can repoint it without a restart (#220).
|
||||
///
|
||||
/// # Errors
|
||||
///
|
||||
/// Fails if the HTTP client cannot be constructed.
|
||||
pub fn with_endpoint(config: OpenAiConfig, endpoint: OpenAiEndpoint) -> Result<Self> {
|
||||
let id = BackendId::new(BACKEND_NAME);
|
||||
let http = Client::builder().build().map_err(|err| Error::Transport {
|
||||
backend: id.clone(),
|
||||
source: Box::new(err),
|
||||
})?;
|
||||
let base_url = base_url.parse().map_err(|err| Error::Malformed {
|
||||
backend: id.clone(),
|
||||
detail: format!("bad base URL: {err}"),
|
||||
})?;
|
||||
Ok(Self {
|
||||
http,
|
||||
base_url,
|
||||
endpoint,
|
||||
config,
|
||||
model: model.into(),
|
||||
id,
|
||||
})
|
||||
}
|
||||
|
||||
/// The cell this backend re-reads for every request.
|
||||
///
|
||||
/// Whoever constructs the backend — the daemon, at startup — hands a
|
||||
/// clone of this to the settings API, which stores `openai_base_url` and
|
||||
/// `openai_model` into it on every edit. Same shape as #219's
|
||||
/// `Command::timeout_cell`.
|
||||
#[must_use]
|
||||
pub fn endpoint(&self) -> OpenAiEndpoint {
|
||||
self.endpoint.clone()
|
||||
}
|
||||
|
||||
async fn check_status(&self, response: reqwest::Response) -> Result<reqwest::Response> {
|
||||
match response.status() {
|
||||
StatusCode::OK => Ok(response),
|
||||
@@ -160,8 +187,11 @@ impl OpenAi {
|
||||
detail: format!("could not encode batch: {err}"),
|
||||
})?;
|
||||
|
||||
// Re-read per call: the settings API writes this cell when the
|
||||
// operator edits `openai_base_url` or `openai_model` (#220).
|
||||
let endpoint = self.endpoint.read();
|
||||
let body = ChatRequest {
|
||||
model: &self.model,
|
||||
model: &endpoint.model,
|
||||
messages: vec![
|
||||
ChatMessage {
|
||||
role: "system",
|
||||
@@ -174,7 +204,7 @@ impl OpenAi {
|
||||
],
|
||||
};
|
||||
|
||||
let url = self
|
||||
let url = endpoint
|
||||
.base_url
|
||||
.join("chat/completions")
|
||||
.map_err(|err| Error::Malformed {
|
||||
@@ -191,7 +221,7 @@ impl OpenAi {
|
||||
request = request.bearer_auth(api_key);
|
||||
}
|
||||
|
||||
tracing::debug!(backend = %self.id, model = %self.model, "OpenAI translate request");
|
||||
tracing::debug!(backend = %self.id, model = %endpoint.model, "OpenAI translate request");
|
||||
let response = request.send().await.map_err(|err| Error::Transport {
|
||||
backend: self.id.clone(),
|
||||
source: Box::new(err),
|
||||
@@ -266,9 +296,12 @@ impl Backend for OpenAi {
|
||||
/// `models` is a read-only listing: reachable and key accepted, nothing
|
||||
/// spent (#200). Endpoints that need no key are probed the same way,
|
||||
/// just without the header.
|
||||
///
|
||||
/// The base URL is read from the live cell, not from a start-up copy, so
|
||||
/// the lamp judges whatever endpoint is configured right now (#220).
|
||||
fn probe(&self) -> ProbeFuture<'_> {
|
||||
Box::pin(async move {
|
||||
let mut url = self.base_url.clone();
|
||||
let mut url = self.endpoint.read().base_url;
|
||||
url.set_path(&format!("{}/models", url.path().trim_end_matches('/')));
|
||||
let mut request = self.http.get(url);
|
||||
if let Some(key) = &self.config.api_key {
|
||||
@@ -357,11 +390,25 @@ mod tests {
|
||||
use arr_core::Language;
|
||||
|
||||
use super::OpenAi;
|
||||
use crate::openai_endpoint::DEFAULT_MODEL;
|
||||
use crate::translate::Backend;
|
||||
use crate::translate::{language_name, strip_code_fence, system_prompt};
|
||||
|
||||
fn backend() -> OpenAi {
|
||||
OpenAi::new("gpt-4o-mini", super::OpenAiConfig { api_key: None }).expect("client builds")
|
||||
OpenAi::new(super::OpenAiConfig { api_key: None }).expect("client builds")
|
||||
}
|
||||
|
||||
/// #220: the backend holds the cell, not a copy — a repoint between two
|
||||
/// calls is what the settings handler relies on.
|
||||
#[test]
|
||||
fn the_backend_follows_its_endpoint_cell() {
|
||||
let backend = backend();
|
||||
let cell = backend.endpoint();
|
||||
assert_eq!(cell.model(), DEFAULT_MODEL);
|
||||
cell.set(Some("http://box:8080/v1/"), Some("llama"))
|
||||
.expect("repoint");
|
||||
assert_eq!(backend.endpoint.model(), "llama");
|
||||
assert_eq!(backend.endpoint.base_url(), "http://box:8080/v1/");
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -0,0 +1,168 @@
|
||||
//! Where the OpenAI-compatible translation backend points, and which model
|
||||
//! it names (`DESIGN.md` §15, issue #220).
|
||||
//!
|
||||
//! Its own module, and not behind `translate-openai`, because the settings
|
||||
//! surface must hold and validate these two values whether or not this build
|
||||
//! compiled the backend in: the `subtitle_settings` columns exist either way,
|
||||
//! and an operator who edits them on a binary without the feature should get
|
||||
//! the same 422 on a base URL that does not parse.
|
||||
|
||||
use std::sync::{Arc, RwLock};
|
||||
|
||||
use reqwest::Url;
|
||||
|
||||
use crate::translate::{BackendId, Error, Result};
|
||||
|
||||
const BACKEND_NAME: &str = "openai";
|
||||
|
||||
/// Where the backend points when `subtitle_settings.openai_base_url` is
|
||||
/// NULL — the row's own comment calls that "the backend's own default".
|
||||
pub const DEFAULT_BASE_URL: &str = "https://api.openai.com/v1/";
|
||||
/// The model named when `subtitle_settings.openai_model` is NULL. Unlike the
|
||||
/// base URL this is a genuine guess rather than an obvious address, so it is
|
||||
/// deliberately the cheap one: an operator who cares picks another.
|
||||
pub const DEFAULT_MODEL: &str = "gpt-4o-mini";
|
||||
|
||||
/// Where an [`OpenAi`](crate::openai::OpenAi) backend is pointed and which model it names, in a
|
||||
/// cell it re-reads for every request (issue #220).
|
||||
///
|
||||
/// Both are `subtitle_settings` rows, not bootstrap config: that backend is
|
||||
/// not `OpenAI` but anything speaking the same request shape — `llama.cpp`,
|
||||
/// a local gateway, a hosted provider — and which one is in use is a thing
|
||||
/// the operator tries and changes (DESIGN.md §15). Whoever constructs the
|
||||
/// backend hands a clone of this to the settings API, which stores the
|
||||
/// edited row into it; that is the same path from database to running
|
||||
/// process #219 built for the remote-command timeout.
|
||||
///
|
||||
/// `None` on either side means the backend's own default —
|
||||
/// [`DEFAULT_BASE_URL`] and [`DEFAULT_MODEL`].
|
||||
#[derive(Clone, Debug)]
|
||||
pub struct OpenAiEndpoint {
|
||||
inner: Arc<RwLock<Endpoint>>,
|
||||
}
|
||||
|
||||
/// One resolved endpoint, cloned out of the lock before any await.
|
||||
#[derive(Clone, Debug)]
|
||||
pub(crate) struct Endpoint {
|
||||
pub(crate) base_url: Url,
|
||||
pub(crate) model: String,
|
||||
}
|
||||
|
||||
/// Resolve a stored pair into a usable endpoint. A blank string is absent,
|
||||
/// not an endpoint named "": the settings form sends an empty field for
|
||||
/// "use the default".
|
||||
fn resolve(base_url: Option<&str>, model: Option<&str>) -> Result<Endpoint> {
|
||||
let raw = base_url
|
||||
.map(str::trim)
|
||||
.filter(|url| !url.is_empty())
|
||||
.unwrap_or(DEFAULT_BASE_URL);
|
||||
// Every path this backend builds is relative to the base, and
|
||||
// `Url::join` drops the last segment of a base that does not end in a
|
||||
// slash — so `http://box:8080/v1` would reach `/chat/completions`
|
||||
// rather than `/v1/chat/completions`. Normalising here means the
|
||||
// operator may type it either way.
|
||||
let normalised = if raw.ends_with('/') {
|
||||
raw.to_owned()
|
||||
} else {
|
||||
format!("{raw}/")
|
||||
};
|
||||
let base_url: Url = normalised.parse().map_err(|err| Error::Malformed {
|
||||
backend: BackendId::new(BACKEND_NAME),
|
||||
detail: format!("bad base URL: {err}"),
|
||||
})?;
|
||||
let model = model
|
||||
.map(str::trim)
|
||||
.filter(|model| !model.is_empty())
|
||||
.unwrap_or(DEFAULT_MODEL)
|
||||
.to_owned();
|
||||
Ok(Endpoint { base_url, model })
|
||||
}
|
||||
|
||||
impl OpenAiEndpoint {
|
||||
/// A cell holding `base_url` and `model`, either of which may be `None`
|
||||
/// for the backend's own default.
|
||||
///
|
||||
/// # Errors
|
||||
///
|
||||
/// [`Error::Malformed`] when `base_url` does not parse.
|
||||
pub fn new(base_url: Option<&str>, model: Option<&str>) -> Result<Self> {
|
||||
Ok(Self {
|
||||
inner: Arc::new(RwLock::new(resolve(base_url, model)?)),
|
||||
})
|
||||
}
|
||||
|
||||
/// Repoint the running backend. Validated before anything is stored, so
|
||||
/// a rejected edit leaves the previous endpoint in place.
|
||||
///
|
||||
/// # Errors
|
||||
///
|
||||
/// [`Error::Malformed`] when `base_url` does not parse.
|
||||
///
|
||||
/// # Panics
|
||||
///
|
||||
/// If a previous writer panicked while holding the lock.
|
||||
pub fn set(&self, base_url: Option<&str>, model: Option<&str>) -> Result<()> {
|
||||
let endpoint = resolve(base_url, model)?;
|
||||
*self.inner.write().expect("endpoint lock poisoned") = endpoint;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// The base URL in force right now, as the backend would use it.
|
||||
#[must_use]
|
||||
pub fn base_url(&self) -> String {
|
||||
self.read().base_url.to_string()
|
||||
}
|
||||
|
||||
/// The model named in force right now.
|
||||
#[must_use]
|
||||
pub fn model(&self) -> String {
|
||||
self.read().model
|
||||
}
|
||||
|
||||
pub(crate) fn read(&self) -> Endpoint {
|
||||
self.inner.read().expect("endpoint lock poisoned").clone()
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{OpenAiEndpoint, DEFAULT_BASE_URL, DEFAULT_MODEL};
|
||||
|
||||
/// #220: NULL columns mean the backend's own default, and a base URL the
|
||||
/// operator typed without a trailing slash still resolves under its own
|
||||
/// path rather than at the host root.
|
||||
#[test]
|
||||
fn an_absent_setting_falls_back_and_a_bare_base_url_keeps_its_path() {
|
||||
let endpoint = OpenAiEndpoint::new(None, None).expect("defaults resolve");
|
||||
assert_eq!(endpoint.base_url(), DEFAULT_BASE_URL);
|
||||
assert_eq!(endpoint.model(), DEFAULT_MODEL);
|
||||
|
||||
// An empty field is "use the default", not an endpoint named "".
|
||||
endpoint.set(Some(" "), Some("")).expect("blanks resolve");
|
||||
assert_eq!(endpoint.base_url(), DEFAULT_BASE_URL);
|
||||
assert_eq!(endpoint.model(), DEFAULT_MODEL);
|
||||
|
||||
endpoint
|
||||
.set(Some("http://127.0.0.1:8080/v1"), Some("local-model"))
|
||||
.expect("a local endpoint resolves");
|
||||
assert_eq!(endpoint.base_url(), "http://127.0.0.1:8080/v1/");
|
||||
assert_eq!(endpoint.model(), "local-model");
|
||||
}
|
||||
|
||||
/// A rejected edit must leave the running backend where it was, rather
|
||||
/// than half-applying the model and dropping the URL.
|
||||
#[test]
|
||||
fn a_base_url_that_does_not_parse_is_refused_and_changes_nothing() {
|
||||
let endpoint =
|
||||
OpenAiEndpoint::new(Some("http://box:8080/v1"), Some("m")).expect("endpoint resolves");
|
||||
let error = endpoint
|
||||
.set(Some("not a url"), Some("other"))
|
||||
.expect_err("a bad base URL is refused");
|
||||
assert!(
|
||||
matches!(error, crate::translate::Error::Malformed { .. }),
|
||||
"got {error:?}"
|
||||
);
|
||||
assert_eq!(endpoint.base_url(), "http://box:8080/v1/");
|
||||
assert_eq!(endpoint.model(), "m");
|
||||
}
|
||||
}
|
||||
@@ -16,7 +16,7 @@ use std::time::Duration;
|
||||
|
||||
use arr_core::Language;
|
||||
use arr_subs::translate::{translate, Backend, Error};
|
||||
use arr_subs::{OpenAi, OpenAiConfig};
|
||||
use arr_subs::{OpenAi, OpenAiConfig, OpenAiEndpoint};
|
||||
use wiremock::matchers::{header, method, path};
|
||||
use wiremock::{Mock, MockServer, ResponseTemplate};
|
||||
|
||||
@@ -319,9 +319,60 @@ async fn an_unsupported_target_is_refused_before_any_request() {
|
||||
assert!(matches!(error, Error::UnsupportedTarget { .. }));
|
||||
}
|
||||
|
||||
/// #220: an edit of `openai_base_url` / `openai_model` reaches the running
|
||||
/// backend without a restart — the next batch goes to the new endpoint and
|
||||
/// names the new model, with no reconstruction in between.
|
||||
#[tokio::test]
|
||||
async fn repointing_the_endpoint_moves_the_next_batch() {
|
||||
let first = MockServer::start().await;
|
||||
let second = MockServer::start().await;
|
||||
mount_reply(
|
||||
&first,
|
||||
ResponseTemplate::new(200).set_body_string(chat_response(
|
||||
r#""[{\"number\": 1, \"text\": \"do primeiro\"}]""#,
|
||||
)),
|
||||
)
|
||||
.await;
|
||||
mount_reply(
|
||||
&second,
|
||||
ResponseTemplate::new(200).set_body_string(chat_response(
|
||||
r#""[{\"number\": 1, \"text\": \"do segundo\"}]""#,
|
||||
)),
|
||||
)
|
||||
.await;
|
||||
|
||||
let endpoint = OpenAiEndpoint::new(Some(&format!("{}/v1/", first.uri())), Some("first-model"))
|
||||
.expect("endpoint resolves");
|
||||
let backend =
|
||||
OpenAi::with_endpoint(OpenAiConfig { api_key: None }, endpoint.clone()).expect("builds");
|
||||
|
||||
let cues = vec![arr_subs::translate::BatchCue {
|
||||
number: 1,
|
||||
text: "from the first".to_owned(),
|
||||
}];
|
||||
let batch = arr_subs::translate::Batch {
|
||||
source: Language::Other("en".to_owned()),
|
||||
target: Language::PortuguesePortugal,
|
||||
cues: cues.clone(),
|
||||
};
|
||||
let out = backend.translate(&batch).await.expect("first batch");
|
||||
assert_eq!(out[0].text, "do primeiro");
|
||||
|
||||
endpoint
|
||||
.set(Some(&format!("{}/v1/", second.uri())), Some("second-model"))
|
||||
.expect("repoint");
|
||||
let out = backend.translate(&batch).await.expect("second batch");
|
||||
assert_eq!(out[0].text, "do segundo");
|
||||
|
||||
let sent = second.received_requests().await.expect("requests recorded");
|
||||
let body: serde_json::Value =
|
||||
serde_json::from_slice(&sent.last().expect("one request").body).expect("json body");
|
||||
assert_eq!(body["model"], "second-model");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_backend_answers_to_openai() {
|
||||
let id = OpenAi::new("gpt-4o-mini", OpenAiConfig { api_key: None })
|
||||
let id = OpenAi::new(OpenAiConfig { api_key: None })
|
||||
.expect("client builds")
|
||||
.id();
|
||||
assert_eq!(id.as_str(), "openai");
|
||||
|
||||
Reference in New Issue
Block a user