diff --git a/crates/arr-daemon/src/config.rs b/crates/arr-daemon/src/config.rs index ba71794..b745446 100644 --- a/crates/arr-daemon/src/config.rs +++ b/crates/arr-daemon/src/config.rs @@ -38,9 +38,12 @@ pub const ENV_OPENSUBTITLES_USERNAME: &str = "ARR_OPENSUBTITLES_USERNAME"; pub const ENV_OPENSUBTITLES_PASSWORD: &str = "ARR_OPENSUBTITLES_PASSWORD"; // Podnapisi takes no credentials (#215): its search and download are // unauthenticated, so there is nothing here for an operator to set. +// The OpenAI-compatible backend's base URL and model are `subtitle_settings` +// rows, not bootstrap keys (#220): that backend is anything speaking the +// shape, and which endpoint is in use is something the operator changes from +// `/settings`. Only the key is here, because §10 keeps secrets out of the +// database — and an endpoint needing no key at all is valid. pub const ENV_TRANSLATE_OPENAI_API_KEY: &str = "ARR_TRANSLATE_OPENAI_API_KEY"; -pub const ENV_TRANSLATE_OPENAI_BASE_URL: &str = "ARR_TRANSLATE_OPENAI_BASE_URL"; -pub const ENV_TRANSLATE_OPENAI_MODEL: &str = "ARR_TRANSLATE_OPENAI_MODEL"; pub const ENV_TRANSLATE_DEEPL_API_KEY: &str = "ARR_TRANSLATE_DEEPL_API_KEY"; pub const ENV_TRANSLATE_DEEPL_BASE_URL: &str = "ARR_TRANSLATE_DEEPL_BASE_URL"; pub const ENV_TRANSLATE_GOOGLE_API_KEY: &str = "ARR_TRANSLATE_GOOGLE_API_KEY"; @@ -70,11 +73,6 @@ pub const DEFAULT_JELLYFIN_URL: &str = "http://localhost:8096"; pub const DEFAULT_NTFY_URL: &str = "http://localhost"; pub const DEFAULT_ALASS_PATH: &str = "alass"; pub const DEFAULT_FFMPEG_PATH: &str = "ffmpeg"; -/// #191 made the model an `OpenAi::new` argument rather than an -/// `OpenAiConfig` field; no other bootstrap key covers it, so this is that -/// key's default. -pub const DEFAULT_TRANSLATE_OPENAI_MODEL: &str = "gpt-4o-mini"; - #[derive(Debug, thiserror::Error)] pub enum ConfigError { #[error("io: {0}")] @@ -122,10 +120,6 @@ struct ConfigFile { #[serde(default)] ntfy_operator_topic: Option, #[serde(default)] - translate_openai_base_url: Option, - #[serde(default)] - translate_openai_model: Option, - #[serde(default)] translate_deepl_base_url: Option, #[serde(default)] translate_google_base_url: Option, @@ -179,8 +173,6 @@ pub struct EnvOverrides { pub opensubtitles_username: Option, pub opensubtitles_password: Option, pub translate_openai_api_key: Option, - pub translate_openai_base_url: Option, - pub translate_openai_model: Option, pub translate_deepl_api_key: Option, pub translate_deepl_base_url: Option, pub translate_google_api_key: Option, @@ -214,8 +206,6 @@ impl EnvOverrides { opensubtitles_username: std::env::var(ENV_OPENSUBTITLES_USERNAME).ok(), opensubtitles_password: std::env::var(ENV_OPENSUBTITLES_PASSWORD).ok(), translate_openai_api_key: std::env::var(ENV_TRANSLATE_OPENAI_API_KEY).ok(), - translate_openai_base_url: std::env::var(ENV_TRANSLATE_OPENAI_BASE_URL).ok(), - translate_openai_model: std::env::var(ENV_TRANSLATE_OPENAI_MODEL).ok(), translate_deepl_api_key: std::env::var(ENV_TRANSLATE_DEEPL_API_KEY).ok(), translate_deepl_base_url: std::env::var(ENV_TRANSLATE_DEEPL_BASE_URL).ok(), translate_google_api_key: std::env::var(ENV_TRANSLATE_GOOGLE_API_KEY).ok(), @@ -261,13 +251,11 @@ pub struct Config { pub opensubtitles_api_key: Option, pub opensubtitles_username: Option, pub opensubtitles_password: Option, + /// §15. `None` is valid: `llama.cpp` and a local gateway serve without + /// authentication. Where that backend points and which model it names + /// are `subtitle_settings` rows, read at start-up and re-read on every + /// edit (#220), not bootstrap config. pub translate_openai_api_key: Option, - /// §15. No default: an OpenAI-compatible endpoint has no universal - /// address the way `DeepL` or Google Translate do. - pub translate_openai_base_url: Option, - /// §15. Unlike the base URL, a model does have a usable default — - /// see [`DEFAULT_TRANSLATE_OPENAI_MODEL`]. - pub translate_openai_model: String, pub translate_deepl_api_key: Option, /// `None` means the backend's own built-in default when it lands (#192). pub translate_deepl_base_url: Option, @@ -294,8 +282,6 @@ struct SubtitleBootstrap { opensubtitles_username: Option, opensubtitles_password: Option, translate_openai_api_key: Option, - translate_openai_base_url: Option, - translate_openai_model: String, translate_deepl_api_key: Option, translate_deepl_base_url: Option, translate_google_api_key: Option, @@ -311,15 +297,6 @@ fn resolve_subtitle_bootstrap(env: &EnvOverrides, file: &ConfigFile) -> Subtitle opensubtitles_username: env.opensubtitles_username.clone(), opensubtitles_password: env.opensubtitles_password.clone(), translate_openai_api_key: env.translate_openai_api_key.clone(), - translate_openai_base_url: env - .translate_openai_base_url - .clone() - .or_else(|| file.translate_openai_base_url.clone()), - translate_openai_model: env - .translate_openai_model - .clone() - .or_else(|| file.translate_openai_model.clone()) - .unwrap_or_else(|| DEFAULT_TRANSLATE_OPENAI_MODEL.to_string()), translate_deepl_api_key: env.translate_deepl_api_key.clone(), translate_deepl_base_url: env .translate_deepl_base_url @@ -434,8 +411,6 @@ impl Config { opensubtitles_username: subtitles.opensubtitles_username, opensubtitles_password: subtitles.opensubtitles_password, translate_openai_api_key: subtitles.translate_openai_api_key, - translate_openai_base_url: subtitles.translate_openai_base_url, - translate_openai_model: subtitles.translate_openai_model, translate_deepl_api_key: subtitles.translate_deepl_api_key, translate_deepl_base_url: subtitles.translate_deepl_base_url, translate_google_api_key: subtitles.translate_google_api_key, @@ -491,10 +466,6 @@ mod tests { assert_eq!(config.jellyfin_api_key, None); assert_eq!(config.opensubtitles_api_key, None); assert_eq!(config.translate_command_template, None); - assert_eq!( - config.translate_openai_model, - DEFAULT_TRANSLATE_OPENAI_MODEL - ); assert_eq!(config.alass_path, PathBuf::from(DEFAULT_ALASS_PATH)); assert_eq!(config.ffmpeg_path, PathBuf::from(DEFAULT_FFMPEG_PATH)); } @@ -613,8 +584,6 @@ prowlarr_url = "http://prowlarr.internal:9696" std::fs::write( &path, r#" -translate_openai_base_url = "http://llm.internal/v1" -translate_openai_model = "local-model" translate_command_template = "ssh box claude -p" alass_path = "/usr/local/bin/alass" "#, @@ -625,11 +594,6 @@ alass_path = "/usr/local/bin/alass" ..EnvOverrides::default() }; let config = Config::resolve(env.clone()).unwrap(); - assert_eq!( - config.translate_openai_base_url.as_deref(), - Some("http://llm.internal/v1") - ); - assert_eq!(config.translate_openai_model, "local-model"); assert_eq!( config.translate_command_template.as_deref(), Some("ssh box claude -p") @@ -646,6 +610,27 @@ alass_path = "/usr/local/bin/alass" assert_eq!(config.ffmpeg_path, PathBuf::from("/opt/bin/ffmpeg")); } + /// #220 retired `translate_openai_base_url` and `translate_openai_model` + /// — both are `subtitle_settings` rows now. `deny_unknown_fields` turns + /// a config file still carrying them into a parse error, so an operator + /// who upgrades sees the move rather than a setting silently ignored. + #[test] + fn the_retired_openai_endpoint_keys_are_a_parse_error() { + for key in ["translate_openai_base_url", "translate_openai_model"] { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("arr.toml"); + std::fs::write(&path, format!("{key} = \"x\"\n")).unwrap(); + let env = EnvOverrides { + config_file: Some(path.to_string_lossy().into_owned()), + ..EnvOverrides::default() + }; + assert!( + matches!(Config::resolve(env), Err(ConfigError::TomlDecode(_))), + "{key} must no longer be accepted" + ); + } + } + #[test] fn tmdb_url_is_an_env_only_seam() { let config = Config::resolve(EnvOverrides::default()).unwrap(); diff --git a/crates/arr-daemon/src/main.rs b/crates/arr-daemon/src/main.rs index 55c45e1..882dfde 100644 --- a/crates/arr-daemon/src/main.rs +++ b/crates/arr-daemon/src/main.rs @@ -141,6 +141,9 @@ fn api_state( if let Some(timeout) = &translators.command_timeout { state = state.with_command_timeout(Arc::clone(timeout)); } + if let Some(endpoint) = &translators.openai_endpoint { + state = state.with_openai_endpoint(endpoint.clone()); + } Ok(state) } @@ -165,7 +168,7 @@ async fn run() -> Result<(), Error> { // lane must see the same backends, or the command translator's live // timeout cell (#219) would fork. let translators = translation_backends(&config); - seed_command_timeout(&database, &translators).await?; + seed_translator_settings(&database, &translators).await?; let (reconcile, manual_grab, manual_tv) = reconcile_loop( &database, &config, @@ -264,27 +267,45 @@ async fn run() -> Result<(), Error> { } } -/// Seed the command translator's live timeout from the settings row, so a -/// restart does not fall back to the compiled-in default until the next -/// settings edit (issue #219). The row is guaranteed to exist and to carry a -/// positive value — migration `0025` seeds it and the column CHECK enforces -/// it. -async fn seed_command_timeout(database: &Db, translators: &Translators) -> Result<(), Error> { - let Some(cell) = &translators.command_timeout else { +/// Seed the translators' live settings from the row, so a restart does not +/// fall back to compiled-in defaults until the next settings edit (issues +/// #219 and #220): the command translator's timeout, and where the +/// OpenAI-compatible backend points plus which model it names. +/// +/// The row is guaranteed to exist — migration `0025` seeds it — and the +/// timeout is guaranteed positive by its column CHECK. The two `OpenAI` +/// columns are nullable, and NULL means the backend's own default. +async fn seed_translator_settings(database: &Db, translators: &Translators) -> Result<(), Error> { + if translators.command_timeout.is_none() && translators.openai_endpoint.is_none() { return Ok(()); - }; - let seconds: i64 = sqlx::query_scalar!( - r#"SELECT remote_command_timeout_seconds AS "remote_command_timeout_seconds!: i64" + } + let row = sqlx::query!( + r#"SELECT 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(database.pool()) .await?; - cell.store( - u64::try_from(seconds) - .unwrap_or(u64::MAX) - .saturating_mul(1_000), - std::sync::atomic::Ordering::Relaxed, - ); + if let Some(cell) = &translators.command_timeout { + cell.store( + u64::try_from(row.remote_command_timeout_seconds) + .unwrap_or(u64::MAX) + .saturating_mul(1_000), + std::sync::atomic::Ordering::Relaxed, + ); + } + if let Some(endpoint) = &translators.openai_endpoint { + // A row that does not parse must not stop the daemon booting: the + // API validates on write, so this only fires for a hand-edited + // database. The backend stays at its default and the health lamp + // says so. + if let Err(error) = + endpoint.set(row.openai_base_url.as_deref(), row.openai_model.as_deref()) + { + tracing::warn!(%error, "stored OpenAI endpoint is unusable; keeping the default"); + } + } Ok(()) } @@ -647,6 +668,10 @@ fn subtitle_providers( struct Translators { backends: Vec>, command_timeout: Option>, + /// The OpenAI-compatible backend's live endpoint (#220), when that + /// backend is compiled in. Its base URL and model are database rows, so + /// this rides along the same way the command timeout does. + openai_endpoint: Option, } #[cfg_attr( @@ -661,22 +686,22 @@ struct Translators { fn translation_backends(config: &Config) -> Translators { let mut backends: Vec> = Vec::new(); let mut command_timeout: Option> = None; + let mut openai_endpoint: Option = None; #[cfg(feature = "translate-openai")] { + // Built at the backend's own defaults and repointed from the + // settings row a moment later (#220). Never gated on the API key: + // `llama.cpp` serves without authentication, so a base URL and no + // key is a valid configuration (DESIGN.md §15). let openai_config = arr_subs::OpenAiConfig { api_key: config.translate_openai_api_key.clone(), }; - let backend = match &config.translate_openai_base_url { - Some(base_url) => arr_subs::OpenAi::with_base_url( - config.translate_openai_model.clone(), - openai_config, - base_url, - ), - None => arr_subs::OpenAi::new(config.translate_openai_model.clone(), openai_config), - }; - match backend { - Ok(backend) => backends.push(std::sync::Arc::new(backend)), + match arr_subs::OpenAi::new(openai_config) { + Ok(backend) => { + openai_endpoint = Some(backend.endpoint()); + backends.push(std::sync::Arc::new(backend)); + } Err(error) => tracing::warn!(%error, "OpenAI-compatible translator not available"), } } @@ -737,5 +762,6 @@ fn translation_backends(config: &Config) -> Translators { Translators { backends, command_timeout, + openai_endpoint, } }