Repository navigation
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A failed attempt can currently publish the pipeline's final error while that task is still retrying. A caller waiting for the pipeline then receives the first failure even though the retry and subsequent steps eventually succeed.
Propagate terminal pipeline errors from
post_saverather thanon_error. Taskiq finishes all retry middleware decisions before saving results and skips both saving and post-save hooks forNoResultErrorattempts. This keeps an in-flight retry from publishing a premature final result, independently of the relative order ofPipelineMiddlewareandSimpleRetryMiddleware. Ordinary failures and exhausted retries still fail the final pipeline task; a failing final step keeps its own saved result. Successful continuation uses the existing path.Fixes #26 (the original premature-result report). The separate comment about delayed SmartRetry scheduling changing
_pipe_dataserialization is not addressed by this patch.Assignment was requested at #26 (comment); no actual assignment has been made. The open group-support PR does not modify this middleware, and no overlapping retry PR was found.
Validation actually run
pytest -q -n 2: 11 passed, the complete repository suite.d5d4da6e1733e2886ef955b505f7818690316587worktree: 3 failed, 5 passed. The two successful-retry orders expose an already-ready final error while the retry waits on an Event; the third failure shows that an explicitly suppressedNoResultErroralso incorrectly marks the pipeline failed. Five controls cover exhausted retries/ordinary terminal errors in both orders, plus the final-step saved result.InMemoryBrokerreceiver/middleware/result path, without replacing those components. Events observe completion of all error hooks and hold the retry deterministically; no timing sleep is used to manufacture the failure.4with exactly two attempts. This uses a real Redis result backend with an in-processInMemoryBrokerexecutor; it is not a separate worker-process or RedisStreamBroker end-to-end test.git diff --cached --checkpassed.Not run locally: the full Python/OS/Pydantic CI matrix, delayed SmartRetry/scheduler serialization, a separate worker process, or result-backend outage/failover.
no_result_on_retry=Falsedeliberately permits attempt results to be saved and is outside the default suppressed-result behavior tested here.Review status
AI-assisted investigation, implementation, and testing. Please keep this PR in Draft pending independent and account-owner review; no completed human review is claimed.