From 285a76f2e2ec398d3858e99fd99015714ce275ca Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Thu, 11 Jun 2026 17:42:20 +0200 Subject: [PATCH] refactor(manager-core): single source for executor-timeout default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stage 6 review Obs 1b: the 300s default lived as a literal in two places (DEFAULT_ASYNC_EXEC_TIMEOUT in dispatcher.rs and DEFAULT_SAFE_VISIBILITY_VS_EXEC_BUDGET_SECS in triggers_api.rs). A future change to the dispatcher would silently leave the visibility-timeout warn threshold out of sync. Extract a single pub const DEFAULT_ASYNC_EXEC_TIMEOUT_SECS: u32 = 300 in dispatcher.rs, derive the existing Duration constant from it, and re-export it under the triggers_api name so the validator's self-contained semantics are preserved at the call site. Tests still inject the value explicitly via TEST_SAFE_LIMIT — no behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/manager-core/src/dispatcher.rs | 8 +++++++- crates/manager-core/src/triggers_api.rs | 9 +++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/crates/manager-core/src/dispatcher.rs b/crates/manager-core/src/dispatcher.rs index 355c55c..7040895 100644 --- a/crates/manager-core/src/dispatcher.rs +++ b/crates/manager-core/src/dispatcher.rs @@ -106,7 +106,13 @@ const DEFAULT_TICK_INTERVAL: Duration = Duration::from_millis(100); /// platform-wide ceiling here. Matches `LocalExecutorClient`'s own /// 5-minute cap. /// F-Q-009: env-overridable via `PICLOUD_DISPATCHER_ASYNC_EXEC_TIMEOUT_SEC`. -const DEFAULT_ASYNC_EXEC_TIMEOUT: Duration = Duration::from_secs(300); +/// +/// `pub` so `triggers_api::validate_queue_visibility_timeout` can use +/// the same default for its warn-threshold — otherwise a change here +/// would silently leave the visibility-timeout warner out of sync. +pub const DEFAULT_ASYNC_EXEC_TIMEOUT_SECS: u32 = 300; +const DEFAULT_ASYNC_EXEC_TIMEOUT: Duration = + Duration::from_secs(DEFAULT_ASYNC_EXEC_TIMEOUT_SECS as u64); /// Read `PICLOUD_DISPATCHER_TICK_INTERVAL_MS`; invalid values fall back /// to the documented default with a tracing-warn. diff --git a/crates/manager-core/src/triggers_api.rs b/crates/manager-core/src/triggers_api.rs index dd94ea3..06e6edf 100644 --- a/crates/manager-core/src/triggers_api.rs +++ b/crates/manager-core/src/triggers_api.rs @@ -41,10 +41,11 @@ use crate::trigger_repo::{ const MIN_QUEUE_VISIBILITY_TIMEOUT_SECS: u32 = 30; /// Default soft-warning ceiling when `PICLOUD_DISPATCHER_ASYNC_EXEC_TIMEOUT_SEC` -/// is unset — matches `DEFAULT_ASYNC_EXEC_TIMEOUT` in dispatcher.rs. -/// Kept as a constant rather than imported so the validator stays -/// self-contained for unit tests. -const DEFAULT_SAFE_VISIBILITY_VS_EXEC_BUDGET_SECS: u32 = 300; +/// is unset. Re-exported from the dispatcher so the single source of +/// truth for the executor-budget default lives in one place — a future +/// change to the dispatcher default shifts the visibility-timeout warn +/// boundary in lockstep. +use crate::dispatcher::DEFAULT_ASYNC_EXEC_TIMEOUT_SECS as DEFAULT_SAFE_VISIBILITY_VS_EXEC_BUDGET_SECS; /// Read the dispatcher's per-message executor budget at validation /// time so the warn-threshold tracks the live env-overridable value