Skip to content

fix: Drive repeating tasks from a delay source - #521

Merged
jsonbailey merged 2 commits into
mainfrom
jb/sdk-2792/delay-source
Sep 16, 2026
Merged

jsonbailey merged 2 commits into
mainfrom
jb/sdk-2792/delay-source

Conversation

@jsonbailey

@jsonbailey jsonbailey commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

BEGIN_COMMIT_OVERRIDE
feat: Drive repeating tasks from a delay source
fix: Repeating task wait duration is no longer reduced by a slow callback duration.
END_COMMIT_OVERRIDE

Summary

Repeating tasks now take their wait from a delay source rather than a fixed interval, and the wait starts when the callback returns rather than when it began.

Groundwork for RETRY-spec conformance, which needs polling to vary its interval after a failure. Nothing varies it yet — every caller here uses the fixed-interval path — so this is a refactor plus one behavior change, reviewable on its own.

Behavior change

The wait now starts when the callback returns. Previously the interval was measured from the start of each invocation and the callback's own duration was subtracted, so a slow callback shortened the following wait. A flush configured every 5 seconds that takes 200ms used to run every 5s; it now runs 5.2s apart.

Bounded by the callback's duration, and poll_interval is already documented as a minimum rather than an exact rate. The reason for changing it: with a variable delay, subtracting the attempt's duration means a slow failure gets less backoff than a fast one, which is backwards — a slow failure more likely means a struggling service.

What changed

  • ldclient/impl/delay.py (new) — a DelaySource protocol with a single read-only property, and FixedDelay for the constant case. It imports nothing but typing, so both schedulers can depend on it and neither owns it.
  • impl/repeating_task.py and impl/aio/concurrency.py — both take a DelaySource, with an at_interval factory for the fixed case. Ten call sites change by one word.
  • Both start() methods are now idempotent, logging and returning instead of raising. AsyncRepeatingTask previously raised RuntimeError; sync inherited Thread.start(), which also raises. The motivation is that AsyncLDClient.start() is documented as an idempotent no-op, so a raise from an internal primitive could surface out of a call the caller was told is safe to repeat.

Testing

make test: 1536 passed. make lint: clean across 226 files.

test_async_polling.py::test_second_start_call_raises became ..._is_a_no_op — it asserted the behavior this change removes.

Gap semantics was verified by measurement rather than by reading: a 0.2s callback on a 0.2s interval now produces 0.41s between invocation starts, where the old arithmetic produced back-to-back runs. No existing test asserted an upper bound on a period, which is the shape that would have broken; the one period assertion is a lower bound, which gap semantics can only satisfy more easily.


Note

Overview
Introduces a shared DelaySource (FixedDelay for constant waits) and refactors sync RepeatingTask and AsyncRepeatingTask to read **next_delay after each callback instead of taking a bare interval. Call sites move to at_interval(...); nothing uses variable delays yet, but the shape supports future retry/backoff behavior.

Timing change: the wait between runs now starts when the callback returns, not when it begins—slow work no longer shortens the following sleep (e.g. a 5s flush plus 200ms work is ~5.2s between starts, not ~5s).

Lifecycle change: start() is idempotent on both paths (log and ignore duplicate starts instead of raising), matching callers like idempotent client start(). Async repeating tasks also log unhandled background failures via a done callback.

Tests add coverage for delay reads, gap semantics, idempotent start, and start-after-stop not resuming.

Reviewed by Cursor Bugbot for commit 8d7515e. Bugbot is set up for automated code reviews on this repo. Configure here.

@jsonbailey
jsonbailey marked this pull request as ready for review September 14, 2026 16:35
@jsonbailey
jsonbailey requested a review from a team as a code owner September 14, 2026 16:35
@jsonbailey jsonbailey changed the title refactor: Drive repeating tasks from a delay source fix: Drive repeating tasks from a delay source Sep 14, 2026
@tanderson-ld
tanderson-ld self-requested a review September 15, 2026 15:11
@jsonbailey jsonbailey changed the title fix: Drive repeating tasks from a delay source fix: Drive repeating tasks from a delay source, wait time now excludes the callback duration Sep 16, 2026
@jsonbailey jsonbailey changed the title fix: Drive repeating tasks from a delay source, wait time now excludes the callback duration fix: Drive repeating tasks from a delay source Sep 16, 2026
@jsonbailey
jsonbailey merged commit 9abe8cb into main Sep 16, 2026
18 checks passed
@jsonbailey
jsonbailey deleted the jb/sdk-2792/delay-source branch September 16, 2026 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants