feat(arr): expose the OpenAI endpoint on /settings
Two more fields on the subtitle settings row, validated on write — a base URL that does not parse is a 422 naming the field — and pushed into the cell the running backend and the health lamp both read.
This commit is contained in:
@@ -6,7 +6,7 @@ use std::sync::{atomic::AtomicU64, Arc};
|
||||
use std::time::Duration;
|
||||
|
||||
use arr_db::Db;
|
||||
use arr_subs::{Backend, Provider, Syncer};
|
||||
use arr_subs::{Backend, OpenAiEndpoint, Provider, Syncer};
|
||||
use tokio::sync::mpsc;
|
||||
|
||||
use crate::jellyfin::JellyfinClient;
|
||||
@@ -90,6 +90,11 @@ pub struct AppState {
|
||||
/// `None` unless the daemon compiled and configured that backend; the
|
||||
/// settings API writes it on every edit of the row.
|
||||
command_timeout: Option<Arc<AtomicU64>>,
|
||||
/// Where the OpenAI-compatible backend points and which model it names
|
||||
/// (#220). `None` unless that backend is compiled in and wired up; the
|
||||
/// settings API repoints it on every edit, and the health lamp probes
|
||||
/// whatever it currently holds.
|
||||
openai_endpoint: Option<OpenAiEndpoint>,
|
||||
/// The configured `ffmpeg` binary, for the health lamps (#200).
|
||||
ffmpeg_binary: OsString,
|
||||
jellyfin: Option<JellyfinClient>,
|
||||
@@ -167,6 +172,7 @@ impl AppState {
|
||||
subtitle_providers: Arc::new(Vec::new()),
|
||||
translation_backends: Arc::new(Vec::new()),
|
||||
command_timeout: None,
|
||||
openai_endpoint: None,
|
||||
ffmpeg_binary: DEFAULT_FFMPEG_BINARY.into(),
|
||||
jellyfin: None,
|
||||
syncer: Syncer::default(),
|
||||
@@ -213,6 +219,15 @@ impl AppState {
|
||||
self
|
||||
}
|
||||
|
||||
/// Attach the cell the OpenAI-compatible backend re-reads per request
|
||||
/// (#220), so an edit of `openai_base_url` or `openai_model` reaches it
|
||||
/// — and the health lamp probes it — without a restart.
|
||||
#[must_use]
|
||||
pub fn with_openai_endpoint(mut self, endpoint: OpenAiEndpoint) -> Self {
|
||||
self.openai_endpoint = Some(endpoint);
|
||||
self
|
||||
}
|
||||
|
||||
/// Attach the Jellyfin client, so a manual subtitle grab or translation
|
||||
/// can trigger the same library refresh import does (§7.5, §15).
|
||||
#[must_use]
|
||||
@@ -245,6 +260,10 @@ impl AppState {
|
||||
self.command_timeout.as_ref()
|
||||
}
|
||||
|
||||
pub(crate) fn openai_endpoint(&self) -> Option<&OpenAiEndpoint> {
|
||||
self.openai_endpoint.as_ref()
|
||||
}
|
||||
|
||||
/// Attach the `alass` binary this deployment runs (§15). Defaults to
|
||||
/// resolving `alass` from `PATH`.
|
||||
#[must_use]
|
||||
|
||||
@@ -29,6 +29,13 @@ pub struct SubtitleSettings {
|
||||
pub provider_daily_budgets: BTreeMap<String, u32>,
|
||||
pub translator_daily_budgets: BTreeMap<String, u32>,
|
||||
pub remote_command_timeout_seconds: u32,
|
||||
/// Where the OpenAI-compatible backend points (#220). `null` means its
|
||||
/// own default, `https://api.openai.com/v1/` — that backend is anything
|
||||
/// speaking the shape, so a `llama.cpp` address belongs here. Not a
|
||||
/// secret: the API key stays in the environment (§10).
|
||||
pub openai_base_url: Option<String>,
|
||||
/// The model that backend names. `null` means its own default.
|
||||
pub openai_model: Option<String>,
|
||||
/// Engines DESIGN.md §15 knows about that this binary compiled in.
|
||||
/// `translation_engine` is always a member of this list or `null` — a
|
||||
/// backend whose cargo feature is missing is never selectable.
|
||||
@@ -48,6 +55,10 @@ pub struct SubtitleSettingsInput {
|
||||
#[serde(default)]
|
||||
pub translator_daily_budgets: BTreeMap<String, u32>,
|
||||
pub remote_command_timeout_seconds: u32,
|
||||
#[serde(default)]
|
||||
pub openai_base_url: Option<String>,
|
||||
#[serde(default)]
|
||||
pub openai_model: Option<String>,
|
||||
}
|
||||
|
||||
impl SubtitleSettingsInput {
|
||||
@@ -91,6 +102,14 @@ impl SubtitleSettingsInput {
|
||||
if self.remote_command_timeout_seconds == 0 {
|
||||
return Err("remote_command_timeout_seconds: must be greater than zero".into());
|
||||
}
|
||||
// #220: an unparseable base URL is rejected here rather than at the
|
||||
// next translation, where it would surface as an engine that has
|
||||
// quietly stopped working. Validated whether or not this build
|
||||
// compiled the backend in — the column exists either way.
|
||||
if let Some(base_url) = blank_to_none(self.openai_base_url.as_deref()) {
|
||||
arr_subs::OpenAiEndpoint::new(Some(base_url), None)
|
||||
.map_err(|error| format!("openai_base_url: {error}"))?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -108,6 +127,10 @@ impl SubtitleSettingsInput {
|
||||
provider_daily_budgets: json(&self.provider_daily_budgets)?,
|
||||
translator_daily_budgets: json(&self.translator_daily_budgets)?,
|
||||
remote_command_timeout_seconds: i64::from(self.remote_command_timeout_seconds),
|
||||
// An empty field means "use the backend's default", which is the
|
||||
// NULL the migration describes — not an endpoint named "".
|
||||
openai_base_url: blank_to_none(self.openai_base_url.as_deref()).map(str::to_owned),
|
||||
openai_model: blank_to_none(self.openai_model.as_deref()).map(str::to_owned),
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -120,6 +143,13 @@ struct SettingsColumns {
|
||||
provider_daily_budgets: String,
|
||||
translator_daily_budgets: String,
|
||||
remote_command_timeout_seconds: i64,
|
||||
openai_base_url: Option<String>,
|
||||
openai_model: Option<String>,
|
||||
}
|
||||
|
||||
/// A field the operator left empty is absent, not an empty setting.
|
||||
fn blank_to_none(value: Option<&str>) -> Option<&str> {
|
||||
value.map(str::trim).filter(|value| !value.is_empty())
|
||||
}
|
||||
|
||||
fn column<T: serde::de::DeserializeOwned>(
|
||||
@@ -145,6 +175,8 @@ impl SettingsColumns {
|
||||
)?,
|
||||
remote_command_timeout_seconds: u32::try_from(self.remote_command_timeout_seconds)
|
||||
.unwrap_or(0),
|
||||
openai_base_url: self.openai_base_url,
|
||||
openai_model: self.openai_model,
|
||||
available_engines: arr_subs::compiled_engines()
|
||||
.into_iter()
|
||||
.map(str::to_owned)
|
||||
@@ -163,7 +195,9 @@ pub(crate) async fn load(state: &AppState) -> Result<SubtitleSettings, ApiError>
|
||||
translation_engine AS "translation_engine: String",
|
||||
provider_daily_budgets AS "provider_daily_budgets!: String",
|
||||
translator_daily_budgets AS "translator_daily_budgets!: String",
|
||||
remote_command_timeout_seconds AS "remote_command_timeout_seconds!: i64"
|
||||
remote_command_timeout_seconds AS "remote_command_timeout_seconds!: i64",
|
||||
openai_base_url AS "openai_base_url: String",
|
||||
openai_model AS "openai_model: String"
|
||||
FROM subtitle_settings WHERE id = 1"#
|
||||
)
|
||||
.fetch_one(pool(state)?)
|
||||
@@ -201,6 +235,8 @@ pub async fn update(
|
||||
let new_wanted = input.wanted_languages.clone();
|
||||
let columns = input.into_columns()?;
|
||||
let timeout_seconds = columns.remote_command_timeout_seconds;
|
||||
let openai_base_url = columns.openai_base_url.clone();
|
||||
let openai_model = columns.openai_model.clone();
|
||||
|
||||
let previous_wanted: String =
|
||||
sqlx::query_scalar!("SELECT wanted_languages FROM subtitle_settings WHERE id = 1")
|
||||
@@ -217,6 +253,7 @@ pub async fn update(
|
||||
wanted_languages = ?, providers_enabled = ?, translation_engine = ?,
|
||||
provider_daily_budgets = ?, translator_daily_budgets = ?,
|
||||
remote_command_timeout_seconds = ?,
|
||||
openai_base_url = ?, openai_model = ?,
|
||||
updated_at = strftime('%Y-%m-%dT%H:%M:%fZ', 'now')
|
||||
WHERE id = 1"#,
|
||||
columns.wanted_languages,
|
||||
@@ -225,6 +262,8 @@ pub async fn update(
|
||||
columns.provider_daily_budgets,
|
||||
columns.translator_daily_budgets,
|
||||
columns.remote_command_timeout_seconds,
|
||||
columns.openai_base_url,
|
||||
columns.openai_model,
|
||||
)
|
||||
.execute(&mut *transaction)
|
||||
.await?;
|
||||
@@ -248,6 +287,14 @@ pub async fn update(
|
||||
std::sync::atomic::Ordering::Relaxed,
|
||||
);
|
||||
}
|
||||
// #220: the same path for the OpenAI-compatible backend. `validate` has
|
||||
// already parsed the base URL, so this cannot fail for a reason the
|
||||
// operator has not been told about.
|
||||
if let Some(endpoint) = state.openai_endpoint() {
|
||||
if let Err(error) = endpoint.set(openai_base_url.as_deref(), openai_model.as_deref()) {
|
||||
tracing::error!(%error, "validated openai endpoint failed to apply");
|
||||
}
|
||||
}
|
||||
Ok(Json(load(&state).await?))
|
||||
}
|
||||
|
||||
@@ -293,6 +340,31 @@ mod tests {
|
||||
(dir, format!("http://{address}"), timeout)
|
||||
}
|
||||
|
||||
/// The same app, with the OpenAI-compatible backend's live endpoint
|
||||
/// attached — what the daemon wires up when that backend is compiled in.
|
||||
async fn application_with_openai() -> (tempfile::TempDir, String, arr_subs::OpenAiEndpoint) {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let database = arr_db::Db::connect(dir.path().join("arr.db"))
|
||||
.await
|
||||
.expect("connect database");
|
||||
database.migrate().await.expect("migrate database");
|
||||
let endpoint = arr_subs::OpenAiEndpoint::new(None, None).expect("defaults resolve");
|
||||
let state = AppState::new(Upstreams::new(
|
||||
"http://127.0.0.1:1".into(),
|
||||
"http://127.0.0.1:1".into(),
|
||||
))
|
||||
.expect("state")
|
||||
.with_database(database)
|
||||
.with_openai_endpoint(endpoint.clone());
|
||||
let listener = tokio::net::TcpListener::bind("127.0.0.1:0")
|
||||
.await
|
||||
.expect("bind");
|
||||
let address = listener.local_addr().expect("address");
|
||||
let app = router(state);
|
||||
tokio::spawn(async move { axum::serve(listener, app).await.expect("serve") });
|
||||
(dir, format!("http://{address}"), endpoint)
|
||||
}
|
||||
|
||||
/// The same app, with the underlying pool exposed so a test can seed or
|
||||
/// inspect rows the API surface does not read back directly.
|
||||
async fn application_with_pool() -> (tempfile::TempDir, String, sqlx::SqlitePool) {
|
||||
@@ -455,6 +527,105 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// #220: the two OpenAI-compatible endpoint fields are ordinary settings
|
||||
/// — they round-trip, and an omitted or empty one reads back as `null`,
|
||||
/// which the backend takes as "use your own default".
|
||||
#[tokio::test]
|
||||
async fn the_openai_endpoint_round_trips_and_blanks_read_back_null() {
|
||||
let (_dir, base) = application().await;
|
||||
let client = reqwest::Client::new();
|
||||
|
||||
let seeded: serde_json::Value = reqwest::get(format!("{base}/api/settings/subtitles"))
|
||||
.await
|
||||
.expect("get settings")
|
||||
.json()
|
||||
.await
|
||||
.expect("settings json");
|
||||
assert!(seeded["openai_base_url"].is_null());
|
||||
assert!(seeded["openai_model"].is_null());
|
||||
|
||||
let mut payload = valid_input();
|
||||
payload["openai_base_url"] = serde_json::json!("http://127.0.0.1:8080/v1");
|
||||
payload["openai_model"] = serde_json::json!("qwen2.5:7b");
|
||||
let updated: serde_json::Value = client
|
||||
.put(format!("{base}/api/settings/subtitles"))
|
||||
.json(&payload)
|
||||
.send()
|
||||
.await
|
||||
.expect("put settings")
|
||||
.json()
|
||||
.await
|
||||
.expect("updated json");
|
||||
assert_eq!(updated["openai_base_url"], "http://127.0.0.1:8080/v1");
|
||||
assert_eq!(updated["openai_model"], "qwen2.5:7b");
|
||||
|
||||
// An emptied field means "back to the default", not an endpoint
|
||||
// named "" — the settings form sends an empty input, not a null.
|
||||
payload["openai_base_url"] = serde_json::json!("");
|
||||
payload["openai_model"] = serde_json::json!(" ");
|
||||
let cleared: serde_json::Value = client
|
||||
.put(format!("{base}/api/settings/subtitles"))
|
||||
.json(&payload)
|
||||
.send()
|
||||
.await
|
||||
.expect("put settings")
|
||||
.json()
|
||||
.await
|
||||
.expect("updated json");
|
||||
assert!(cleared["openai_base_url"].is_null());
|
||||
assert!(cleared["openai_model"].is_null());
|
||||
}
|
||||
|
||||
/// #220: a base URL that does not parse is a 422 naming the field, the
|
||||
/// same shape `translation_engine` already rejects with.
|
||||
#[tokio::test]
|
||||
async fn a_base_url_that_does_not_parse_is_a_422_naming_the_field() {
|
||||
let (_dir, base) = application().await;
|
||||
let mut payload = valid_input();
|
||||
payload["openai_base_url"] = serde_json::json!("not a url");
|
||||
|
||||
let response = reqwest::Client::new()
|
||||
.put(format!("{base}/api/settings/subtitles"))
|
||||
.json(&payload)
|
||||
.send()
|
||||
.await
|
||||
.expect("put settings");
|
||||
assert_eq!(response.status(), StatusCode::UNPROCESSABLE_ENTITY);
|
||||
let body: serde_json::Value = response.json().await.expect("error json");
|
||||
assert!(
|
||||
body["error"]
|
||||
.as_str()
|
||||
.expect("error string")
|
||||
.starts_with("openai_base_url:"),
|
||||
"got {body}"
|
||||
);
|
||||
}
|
||||
|
||||
/// #220, the point of the issue: the row alone never reaches the running
|
||||
/// backend, so a PUT must repoint the cell it re-reads per request —
|
||||
/// which is also what the health lamp probes.
|
||||
#[tokio::test]
|
||||
async fn a_put_repoints_the_live_openai_endpoint() {
|
||||
let (_dir, base, endpoint) = application_with_openai().await;
|
||||
assert_eq!(endpoint.base_url(), arr_subs::OPENAI_DEFAULT_BASE_URL);
|
||||
assert_eq!(endpoint.model(), arr_subs::OPENAI_DEFAULT_MODEL);
|
||||
|
||||
let mut payload = valid_input();
|
||||
payload["openai_base_url"] = serde_json::json!("http://127.0.0.1:8080/v1");
|
||||
payload["openai_model"] = serde_json::json!("qwen2.5:7b");
|
||||
reqwest::Client::new()
|
||||
.put(format!("{base}/api/settings/subtitles"))
|
||||
.json(&payload)
|
||||
.send()
|
||||
.await
|
||||
.expect("put settings")
|
||||
.error_for_status()
|
||||
.expect("valid input accepted");
|
||||
|
||||
assert_eq!(endpoint.base_url(), "http://127.0.0.1:8080/v1/");
|
||||
assert_eq!(endpoint.model(), "qwen2.5:7b");
|
||||
}
|
||||
|
||||
/// Whether a known engine is selectable depends on which `translate-*`
|
||||
/// features this binary was built with, so the test asks the build rather
|
||||
/// than assuming. With no feature on, every engine is uncompiled and must
|
||||
|
||||
Reference in New Issue
Block a user