feat: make global default timeout 0 (infinite) instead of required 30s
DefaultTimeoutSeconds now means "no timeout" when 0/empty, and that is the new default, rather than an invalid config forcing a positive value. runner.RunJob avoids context.WithTimeout with a zero duration (which would expire immediately) and instead runs on a plain cancelable context when no timeout is configured. Per-job TimeoutSeconds inherit semantics are unchanged.
This commit is contained in:
+4
-1
@@ -105,11 +105,14 @@ func DisplayOverlapPolicy(job domain.Job, globalPolicy domain.OverlapPolicy) str
|
||||
// DisplayTimeout formats a job's effective run timeout for the details panel.
|
||||
// When the job sets its own TimeoutSeconds it is shown as-is; when 0 (inherit),
|
||||
// the global default is shown with "(global default)" appended, mirroring
|
||||
// DisplayOverlapPolicy.
|
||||
// DisplayOverlapPolicy. A non-positive global default means no timeout at all.
|
||||
func DisplayTimeout(job domain.Job, globalDefault int) string {
|
||||
if job.TimeoutSeconds > 0 {
|
||||
return fmt.Sprintf("%d s", job.TimeoutSeconds)
|
||||
}
|
||||
if globalDefault <= 0 {
|
||||
return "no timeout (global default)"
|
||||
}
|
||||
return fmt.Sprintf("%d s (global default)", globalDefault)
|
||||
}
|
||||
|
||||
|
||||
@@ -173,4 +173,7 @@ func TestDisplayTimeout(t *testing.T) {
|
||||
if got, want := DisplayTimeout(inherit, 30), "30 s (global default)"; got != want {
|
||||
t.Errorf("inherited timeout = %q, want %q", got, want)
|
||||
}
|
||||
if got, want := DisplayTimeout(inherit, 0), "no timeout (global default)"; got != want {
|
||||
t.Errorf("inherited infinite timeout = %q, want %q", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -393,8 +393,8 @@ func validateConfig(config domain.Config) error {
|
||||
if config.OverlapPolicy != domain.OverlapPolicySkip && config.OverlapPolicy != domain.OverlapPolicyQueue {
|
||||
return errors.New("overlap policy must be 'skip' or 'queue'")
|
||||
}
|
||||
if config.DefaultTimeoutSeconds <= 0 {
|
||||
return errors.New("default timeout must be a positive number of seconds")
|
||||
if config.DefaultTimeoutSeconds < 0 {
|
||||
return errors.New("default timeout must not be negative (0 means no timeout)")
|
||||
}
|
||||
// Empty Theme is accepted and normalized to the default on load, so older
|
||||
// configs (and hand-built ones) stay valid without an explicit theme.
|
||||
|
||||
@@ -527,7 +527,7 @@ func TestUpdateSettingsRejectsInvalidConfigs(t *testing.T) {
|
||||
{"missing logs dir", func(c *domain.Config) { c.LogsDir = "" }},
|
||||
{"non-positive max files", func(c *domain.Config) { c.MaxLogFiles = 0 }},
|
||||
{"non-positive max age", func(c *domain.Config) { c.MaxLogAgeDays = -1 }},
|
||||
{"non-positive default timeout", func(c *domain.Config) { c.DefaultTimeoutSeconds = 0 }},
|
||||
{"negative default timeout", func(c *domain.Config) { c.DefaultTimeoutSeconds = -1 }},
|
||||
}
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
|
||||
+2
-1
@@ -214,7 +214,8 @@ func (s *Service) effectiveOverlapPolicy(job *domain.Job) domain.OverlapPolicy {
|
||||
// job's own TimeoutSeconds when positive, otherwise the global
|
||||
// Config.DefaultTimeoutSeconds. A non-positive Job.TimeoutSeconds means "inherit
|
||||
// the global default", which is why normalizeJob leaves 0 rather than
|
||||
// backfilling the configured value. The caller must hold mu.
|
||||
// backfilling the configured value. A resolved duration of 0 means no timeout;
|
||||
// runner.RunJob treats it as "run without a deadline". The caller must hold mu.
|
||||
func (s *Service) effectiveTimeout(job *domain.Job) time.Duration {
|
||||
secs := job.TimeoutSeconds
|
||||
if secs <= 0 {
|
||||
|
||||
@@ -633,4 +633,9 @@ func TestEffectiveTimeout(t *testing.T) {
|
||||
if got, want := svc.effectiveTimeout(own), 5*time.Second; got != want {
|
||||
t.Errorf("per-job timeout = %s, want %s", got, want)
|
||||
}
|
||||
|
||||
svc.store.Config.DefaultTimeoutSeconds = 0
|
||||
if got, want := svc.effectiveTimeout(inherit), time.Duration(0); got != want {
|
||||
t.Errorf("inherited timeout with no global default = %s, want %s (no timeout)", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -54,7 +54,8 @@ type Config struct {
|
||||
ExecutionMode ExecutionMode `json:"execution_mode,omitempty"`
|
||||
OverlapPolicy OverlapPolicy `json:"overlap_policy,omitempty"`
|
||||
// DefaultTimeoutSeconds is the run timeout applied to jobs that do not set
|
||||
// their own Job.TimeoutSeconds. It carries the formerly hard-coded 30s guard.
|
||||
// their own Job.TimeoutSeconds. 0 (the default) means no timeout: such jobs
|
||||
// run to completion however long that takes.
|
||||
DefaultTimeoutSeconds int `json:"default_timeout_seconds,omitempty"`
|
||||
Paused bool `json:"paused,omitempty"`
|
||||
// Theme selects the visual appearance. Empty is treated as ThemeDefault so
|
||||
@@ -77,7 +78,7 @@ func DefaultConfig() Config {
|
||||
ExecutionMode: ExecutionModeParallel,
|
||||
OverlapPolicy: OverlapPolicySkip,
|
||||
Theme: ThemeDefault,
|
||||
DefaultTimeoutSeconds: 30,
|
||||
DefaultTimeoutSeconds: 0,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -18,5 +18,6 @@ type Job struct {
|
||||
// TimeoutSeconds bounds how long a run may take before it is killed. 0 means
|
||||
// "inherit the global Config.DefaultTimeoutSeconds", mirroring OverlapPolicy:
|
||||
// normalizeJobs must leave 0 untouched rather than backfilling the default.
|
||||
// The inherited global default may itself be 0, meaning no timeout at all.
|
||||
TimeoutSeconds int `json:"timeout_seconds,omitempty"`
|
||||
}
|
||||
|
||||
+11
-2
@@ -18,8 +18,17 @@ func RunJob(ctx context.Context, job *domain.Job, trigger string, logsDir string
|
||||
started := time.Now()
|
||||
// Commands can hang forever if a script waits for input or a child process
|
||||
// stalls. The effective timeout is resolved by the caller (per-job value or
|
||||
// the global default), keeping the runner ignorant of the global config.
|
||||
runCtx, cancel := context.WithTimeout(ctx, timeout)
|
||||
// the global default), keeping the runner ignorant of the global config. A
|
||||
// non-positive timeout means "no timeout": context.WithTimeout(ctx, 0) would
|
||||
// expire immediately, so fall back to a plain cancelable context that only
|
||||
// ever ends via ctx (e.g. app shutdown).
|
||||
var runCtx context.Context
|
||||
var cancel context.CancelFunc
|
||||
if timeout > 0 {
|
||||
runCtx, cancel = context.WithTimeout(ctx, timeout)
|
||||
} else {
|
||||
runCtx, cancel = context.WithCancel(ctx)
|
||||
}
|
||||
defer cancel()
|
||||
|
||||
var output string
|
||||
|
||||
@@ -376,6 +376,31 @@ func TestRunJobTimesOut(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunJobZeroTimeoutMeansNoTimeout(t *testing.T) {
|
||||
command := "sh"
|
||||
arguments := "-c\nsleep 0.2"
|
||||
if runtime.GOOS == "windows" {
|
||||
command = `C:\Windows\System32\cmd.exe`
|
||||
arguments = "/C\nping -n 2 127.0.0.1 >NUL"
|
||||
}
|
||||
job := domain.Job{
|
||||
ID: 52,
|
||||
Name: "No Timeout Test",
|
||||
Command: command,
|
||||
Arguments: arguments,
|
||||
}
|
||||
|
||||
// A non-positive timeout must not expire immediately (context.WithTimeout
|
||||
// with a zero duration would); the job must run to completion.
|
||||
record, err := RunJob(context.Background(), &job, "Manual", t.TempDir(), 0)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if record.State != "OK" {
|
||||
t.Fatalf("expected job with no timeout to complete OK, got state %q detail %q", record.State, record.Detail)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunJobStartOnlyIgnoresTimeout(t *testing.T) {
|
||||
command := "sh"
|
||||
arguments := "-c\nsleep 5"
|
||||
|
||||
@@ -168,8 +168,8 @@ func TestLoadOrCreateConfigCreatesDefaultsOnFirstRun(t *testing.T) {
|
||||
if got.MaxLogAgeDays != 30 {
|
||||
t.Errorf("default MaxLogAgeDays = %d, want 30", got.MaxLogAgeDays)
|
||||
}
|
||||
if got.DefaultTimeoutSeconds != 30 {
|
||||
t.Errorf("default DefaultTimeoutSeconds = %d, want 30", got.DefaultTimeoutSeconds)
|
||||
if got.DefaultTimeoutSeconds != 0 {
|
||||
t.Errorf("default DefaultTimeoutSeconds = %d, want 0 (no timeout)", got.DefaultTimeoutSeconds)
|
||||
}
|
||||
if got.Theme != domain.ThemeDefault {
|
||||
t.Errorf("default Theme = %q, want %q", got.Theme, domain.ThemeDefault)
|
||||
|
||||
@@ -86,6 +86,7 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject {
|
||||
overlapPolicySelect.SetSelected(string(store.Config.OverlapPolicy))
|
||||
overlapPolicySelect.OnChanged = func(string) { updateSaveState() }
|
||||
defaultTimeout := widget.NewEntry()
|
||||
defaultTimeout.SetPlaceHolder("0 = no timeout")
|
||||
defaultTimeout.SetText(strconv.Itoa(store.Config.DefaultTimeoutSeconds))
|
||||
defaultTimeout.OnChanged = func(string) { updateSaveState() }
|
||||
jobsDir := widget.NewEntry()
|
||||
@@ -132,8 +133,8 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject {
|
||||
return
|
||||
}
|
||||
timeout, err := strconv.Atoi(strings.TrimSpace(defaultTimeout.Text))
|
||||
if err != nil || timeout <= 0 {
|
||||
settingsStatus.SetText("Default timeout must be a positive number")
|
||||
if err != nil || timeout < 0 {
|
||||
settingsStatus.SetText("Default timeout must not be negative (0 = no timeout)")
|
||||
return
|
||||
}
|
||||
// Build the new config from the form and hand it to the Service, which
|
||||
|
||||
Reference in New Issue
Block a user