fix(actions): retry rejected action records - #7892
Open
1fanwang wants to merge 2 commits into
Open
Conversation
|
This PR was flagged by our automated quality checks. If you're a genuine We appreciate your contribution and apologize if this is a false positive! |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a reliability gap in the Actions k8s client where InternalRunService.RecordAction failures can be returned as a successful transport response with a non-OK google.rpc.Status in the response body—previously causing rejected actions to be incorrectly deduplicated and never retried.
Changes:
- Capture
RecordActionresponses innotifyRunServiceand only memoize the action key when the response-body status is OK. - Log response-body rejection status details when the run service rejects an action.
- Add unit tests covering both “rejected stays retryable” and “accepted deduplicates” behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| actions/k8s/client.go | Checks RecordAction response-body status before adding to the dedup filter; logs rejections. |
| actions/k8s/client_test.go | Adds tests ensuring rejected records are retried and accepted records are deduplicated. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
1fanwang
changed the title
RecordAction reports application rejections in its body while returning a nil transport error. Memoizing those keys drops all retries, leaving rejected actions unrecorded. Only an explicit OK response is safe to memoize. Signed-off-by: 1fanwang <1fannnw@gmail.com>
1fanwang
force-pushed
the
record-action-check-body-status
branch
from
August 21, 2026 18:49
215e037 to
3d0cb91
Compare
Comment on lines
+59
to
+63
| func acceptedRecordActionResponse() *connect.Response[workflow.RecordActionResponse] { | ||
| return connect.NewResponse(&workflow.RecordActionResponse{ | ||
| Status: &status.Status{Code: int32(code.Code_OK)}, | ||
| }) | ||
| } |
Signed-off-by: 1fanwang <1fannnw@gmail.com>
Comment on lines
+59
to
+67
| func acceptedRecordActionResponse() *connect.Response[workflow.RecordActionResponse] { | ||
| return connect.NewResponse(&workflow.RecordActionResponse{ | ||
| ActionId: &common.ActionIdentifier{ | ||
| Run: &common.RunIdentifier{Org: "org", Project: "proj", Domain: "dev", Name: "run"}, | ||
| Name: "action", | ||
| }, | ||
| Status: &status.Status{Code: int32(code.Code_OK)}, | ||
| }) | ||
| } |
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.
Why are the changes needed?
An action can disappear from Flyte's run history. The run service rejects the record,
but the client treats the request as successful and never sends it again.
Before this change, rejected or incomplete responses enter the dedup filter. After
this change, only an explicit OK status is memoized, so every other response is retried.
What changes were proposed in this pull request?
Check the response status before updating the dedup filter. Log the returned code and
message for rejections. A nil response, message, or status is treated as a failed
record attempt.
How was this patch tested?
The tests were kept in place while only the implementation file was switched between
the base branch and this PR.
Labels
fixed
Setup process
N/A
Screenshots
N/A
Check all the applicable boxes
Related PRs
N/A
Stack
Docs link
N/A