Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(webapi): Ensure cache deletion on abort workflow #5235

Merged
merged 3 commits into from
Apr 22, 2024

Conversation

pingsutw
Copy link
Member

Tracking issue

NA

Why are the changes needed?

Propeller keeps sending the requests to agents because propeller didn't remove the item from the async cache when users aborted the workflow.

What changes were proposed in this pull request?

Delete the item from the cache on abort.

How was this patch tested?

local sandbox

Setup process

remote cluster

Screenshots

Check all the applicable boxes

  • I updated the documentation accordingly.
  • All new and existing tests passed.
  • All commits are signed-off.

Related PRs

NA

Docs link

NA

@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. bug Something isn't working labels Apr 15, 2024
Comment on lines 102 to 108
cacheItemID := tCtx.TaskExecutionMetadata().GetTaskExecutionID().GetGeneratedName()
err := c.cache.DeleteDelayed(cacheItemID)
if err != nil {
logger.Errorf(ctx, "Failed to delete resource [%v] from cache. Error: %v", cacheItemID, err)
return err
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we do this in Finalize? so that it happens all the time

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Signed-off-by: Kevin Su <pingsutw@apache.org>
EngHabu
EngHabu previously approved these changes Apr 22, 2024
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Apr 22, 2024
Signed-off-by: Kevin Su <pingsutw@apache.org>
@pingsutw pingsutw merged commit 0ffab4a into master Apr 22, 2024
46 checks passed
@pingsutw pingsutw deleted the remove-item-from-cache branch April 22, 2024 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lgtm This PR has been approved by a maintainer size:S This PR changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants