refactor(arr): retire the OpenAI bootstrap keys

#216 added translate_openai_model as an explicit stopgap; the database row
replaces it, along with translate_openai_base_url. Only the API key stays
in the environment. deny_unknown_fields makes a config file still carrying
either one a parse error, so the move is visible rather than ignored.
This commit is contained in:
Miguel Palhas
2026-08-25 08:29:59 +01:00
parent e8bc766d4b
commit 5aa914f81c
2 changed files with 83 additions and 72 deletions
+30 -45
View File
@@ -38,9 +38,12 @@ pub const ENV_OPENSUBTITLES_USERNAME: &str = "ARR_OPENSUBTITLES_USERNAME";
pub const ENV_OPENSUBTITLES_PASSWORD: &str = "ARR_OPENSUBTITLES_PASSWORD"; pub const ENV_OPENSUBTITLES_PASSWORD: &str = "ARR_OPENSUBTITLES_PASSWORD";
// Podnapisi takes no credentials (#215): its search and download are // Podnapisi takes no credentials (#215): its search and download are
// unauthenticated, so there is nothing here for an operator to set. // 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_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_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_DEEPL_BASE_URL: &str = "ARR_TRANSLATE_DEEPL_BASE_URL";
pub const ENV_TRANSLATE_GOOGLE_API_KEY: &str = "ARR_TRANSLATE_GOOGLE_API_KEY"; 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_NTFY_URL: &str = "http://localhost";
pub const DEFAULT_ALASS_PATH: &str = "alass"; pub const DEFAULT_ALASS_PATH: &str = "alass";
pub const DEFAULT_FFMPEG_PATH: &str = "ffmpeg"; 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)] #[derive(Debug, thiserror::Error)]
pub enum ConfigError { pub enum ConfigError {
#[error("io: {0}")] #[error("io: {0}")]
@@ -122,10 +120,6 @@ struct ConfigFile {
#[serde(default)] #[serde(default)]
ntfy_operator_topic: Option<String>, ntfy_operator_topic: Option<String>,
#[serde(default)] #[serde(default)]
translate_openai_base_url: Option<String>,
#[serde(default)]
translate_openai_model: Option<String>,
#[serde(default)]
translate_deepl_base_url: Option<String>, translate_deepl_base_url: Option<String>,
#[serde(default)] #[serde(default)]
translate_google_base_url: Option<String>, translate_google_base_url: Option<String>,
@@ -179,8 +173,6 @@ pub struct EnvOverrides {
pub opensubtitles_username: Option<String>, pub opensubtitles_username: Option<String>,
pub opensubtitles_password: Option<String>, pub opensubtitles_password: Option<String>,
pub translate_openai_api_key: Option<String>, pub translate_openai_api_key: Option<String>,
pub translate_openai_base_url: Option<String>,
pub translate_openai_model: Option<String>,
pub translate_deepl_api_key: Option<String>, pub translate_deepl_api_key: Option<String>,
pub translate_deepl_base_url: Option<String>, pub translate_deepl_base_url: Option<String>,
pub translate_google_api_key: Option<String>, pub translate_google_api_key: Option<String>,
@@ -214,8 +206,6 @@ impl EnvOverrides {
opensubtitles_username: std::env::var(ENV_OPENSUBTITLES_USERNAME).ok(), opensubtitles_username: std::env::var(ENV_OPENSUBTITLES_USERNAME).ok(),
opensubtitles_password: std::env::var(ENV_OPENSUBTITLES_PASSWORD).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_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_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_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(), 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<String>, pub opensubtitles_api_key: Option<String>,
pub opensubtitles_username: Option<String>, pub opensubtitles_username: Option<String>,
pub opensubtitles_password: Option<String>, pub opensubtitles_password: Option<String>,
/// §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<String>, pub translate_openai_api_key: Option<String>,
/// §15. No default: an OpenAI-compatible endpoint has no universal
/// address the way `DeepL` or Google Translate do.
pub translate_openai_base_url: Option<String>,
/// §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<String>, pub translate_deepl_api_key: Option<String>,
/// `None` means the backend's own built-in default when it lands (#192). /// `None` means the backend's own built-in default when it lands (#192).
pub translate_deepl_base_url: Option<String>, pub translate_deepl_base_url: Option<String>,
@@ -294,8 +282,6 @@ struct SubtitleBootstrap {
opensubtitles_username: Option<String>, opensubtitles_username: Option<String>,
opensubtitles_password: Option<String>, opensubtitles_password: Option<String>,
translate_openai_api_key: Option<String>, translate_openai_api_key: Option<String>,
translate_openai_base_url: Option<String>,
translate_openai_model: String,
translate_deepl_api_key: Option<String>, translate_deepl_api_key: Option<String>,
translate_deepl_base_url: Option<String>, translate_deepl_base_url: Option<String>,
translate_google_api_key: Option<String>, translate_google_api_key: Option<String>,
@@ -311,15 +297,6 @@ fn resolve_subtitle_bootstrap(env: &EnvOverrides, file: &ConfigFile) -> Subtitle
opensubtitles_username: env.opensubtitles_username.clone(), opensubtitles_username: env.opensubtitles_username.clone(),
opensubtitles_password: env.opensubtitles_password.clone(), opensubtitles_password: env.opensubtitles_password.clone(),
translate_openai_api_key: env.translate_openai_api_key.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_api_key: env.translate_deepl_api_key.clone(),
translate_deepl_base_url: env translate_deepl_base_url: env
.translate_deepl_base_url .translate_deepl_base_url
@@ -434,8 +411,6 @@ impl Config {
opensubtitles_username: subtitles.opensubtitles_username, opensubtitles_username: subtitles.opensubtitles_username,
opensubtitles_password: subtitles.opensubtitles_password, opensubtitles_password: subtitles.opensubtitles_password,
translate_openai_api_key: subtitles.translate_openai_api_key, 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_api_key: subtitles.translate_deepl_api_key,
translate_deepl_base_url: subtitles.translate_deepl_base_url, translate_deepl_base_url: subtitles.translate_deepl_base_url,
translate_google_api_key: subtitles.translate_google_api_key, 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.jellyfin_api_key, None);
assert_eq!(config.opensubtitles_api_key, None); assert_eq!(config.opensubtitles_api_key, None);
assert_eq!(config.translate_command_template, 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.alass_path, PathBuf::from(DEFAULT_ALASS_PATH));
assert_eq!(config.ffmpeg_path, PathBuf::from(DEFAULT_FFMPEG_PATH)); assert_eq!(config.ffmpeg_path, PathBuf::from(DEFAULT_FFMPEG_PATH));
} }
@@ -613,8 +584,6 @@ prowlarr_url = "http://prowlarr.internal:9696"
std::fs::write( std::fs::write(
&path, &path,
r#" r#"
translate_openai_base_url = "http://llm.internal/v1"
translate_openai_model = "local-model"
translate_command_template = "ssh box claude -p" translate_command_template = "ssh box claude -p"
alass_path = "/usr/local/bin/alass" alass_path = "/usr/local/bin/alass"
"#, "#,
@@ -625,11 +594,6 @@ alass_path = "/usr/local/bin/alass"
..EnvOverrides::default() ..EnvOverrides::default()
}; };
let config = Config::resolve(env.clone()).unwrap(); 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!( assert_eq!(
config.translate_command_template.as_deref(), config.translate_command_template.as_deref(),
Some("ssh box claude -p") 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")); 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] #[test]
fn tmdb_url_is_an_env_only_seam() { fn tmdb_url_is_an_env_only_seam() {
let config = Config::resolve(EnvOverrides::default()).unwrap(); let config = Config::resolve(EnvOverrides::default()).unwrap();
+53 -27
View File
@@ -141,6 +141,9 @@ fn api_state(
if let Some(timeout) = &translators.command_timeout { if let Some(timeout) = &translators.command_timeout {
state = state.with_command_timeout(Arc::clone(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) Ok(state)
} }
@@ -165,7 +168,7 @@ async fn run() -> Result<(), Error> {
// lane must see the same backends, or the command translator's live // lane must see the same backends, or the command translator's live
// timeout cell (#219) would fork. // timeout cell (#219) would fork.
let translators = translation_backends(&config); 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( let (reconcile, manual_grab, manual_tv) = reconcile_loop(
&database, &database,
&config, &config,
@@ -264,27 +267,45 @@ async fn run() -> Result<(), Error> {
} }
} }
/// Seed the command translator's live timeout from the settings row, so a /// Seed the translators' live settings from the row, so a restart does not
/// restart does not fall back to the compiled-in default until the next /// fall back to compiled-in defaults until the next settings edit (issues
/// settings edit (issue #219). The row is guaranteed to exist and to carry a /// #219 and #220): the command translator's timeout, and where the
/// positive value — migration `0025` seeds it and the column CHECK enforces /// OpenAI-compatible backend points plus which model it names.
/// it. ///
async fn seed_command_timeout(database: &Db, translators: &Translators) -> Result<(), Error> { /// The row is guaranteed to exist — migration `0025` seeds it — and the
let Some(cell) = &translators.command_timeout else { /// 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(()); return Ok(());
}; }
let seconds: i64 = sqlx::query_scalar!( let row = sqlx::query!(
r#"SELECT remote_command_timeout_seconds AS "remote_command_timeout_seconds!: i64" 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"# FROM subtitle_settings WHERE id = 1"#
) )
.fetch_one(database.pool()) .fetch_one(database.pool())
.await?; .await?;
cell.store( if let Some(cell) = &translators.command_timeout {
u64::try_from(seconds) cell.store(
.unwrap_or(u64::MAX) u64::try_from(row.remote_command_timeout_seconds)
.saturating_mul(1_000), .unwrap_or(u64::MAX)
std::sync::atomic::Ordering::Relaxed, .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(()) Ok(())
} }
@@ -647,6 +668,10 @@ fn subtitle_providers(
struct Translators { struct Translators {
backends: Vec<std::sync::Arc<dyn arr_subs::Backend>>, backends: Vec<std::sync::Arc<dyn arr_subs::Backend>>,
command_timeout: Option<Arc<AtomicU64>>, command_timeout: Option<Arc<AtomicU64>>,
/// 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<arr_subs::OpenAiEndpoint>,
} }
#[cfg_attr( #[cfg_attr(
@@ -661,22 +686,22 @@ struct Translators {
fn translation_backends(config: &Config) -> Translators { fn translation_backends(config: &Config) -> Translators {
let mut backends: Vec<std::sync::Arc<dyn arr_subs::Backend>> = Vec::new(); let mut backends: Vec<std::sync::Arc<dyn arr_subs::Backend>> = Vec::new();
let mut command_timeout: Option<Arc<AtomicU64>> = None; let mut command_timeout: Option<Arc<AtomicU64>> = None;
let mut openai_endpoint: Option<arr_subs::OpenAiEndpoint> = None;
#[cfg(feature = "translate-openai")] #[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 { let openai_config = arr_subs::OpenAiConfig {
api_key: config.translate_openai_api_key.clone(), api_key: config.translate_openai_api_key.clone(),
}; };
let backend = match &config.translate_openai_base_url { match arr_subs::OpenAi::new(openai_config) {
Some(base_url) => arr_subs::OpenAi::with_base_url( Ok(backend) => {
config.translate_openai_model.clone(), openai_endpoint = Some(backend.endpoint());
openai_config, backends.push(std::sync::Arc::new(backend));
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)),
Err(error) => tracing::warn!(%error, "OpenAI-compatible translator not available"), Err(error) => tracing::warn!(%error, "OpenAI-compatible translator not available"),
} }
} }
@@ -737,5 +762,6 @@ fn translation_backends(config: &Config) -> Translators {
Translators { Translators {
backends, backends,
command_timeout, command_timeout,
openai_endpoint,
} }
} }