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

remove current key from queue after sync #7

Merged
merged 1 commit into from
Aug 31, 2022
Merged

remove current key from queue after sync #7

merged 1 commit into from
Aug 31, 2022

Conversation

ecordell
Copy link
Contributor

@ecordell ecordell commented Aug 31, 2022

If you forget to call done after syncing an key, future Adds to the queue will mark it as dirty without re-adding it to the queue (effectively, the object is processed once until finished, and then never touched again until the operator is restarted).

If you're using the OwnedResourceController, this change makes it so that you don't have to call done from within the sync function; the controller will do it for you after the sync returns.

@@ -163,8 163,7 @@ func (c *OwnedResourceController) processNext(ctx context.Context) bool {
ctx = c.OperationsContext.WithValue(ctx, queue.NewOperations(done, requeue))

c.sync(ctx, *gvr, namespace, name)

cancel()
done()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

done calls cancel already

@ecordell ecordell enabled auto-merge August 31, 2022 19:11
Copy link
Member

@josephschorr josephschorr left a comment

Choose a reason for hiding this comment

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

LGTM

@ecordell ecordell merged commit 23f5716 into main Aug 31, 2022
@ecordell ecordell deleted the queue-done branch August 31, 2022 20:16
@github-actions github-actions bot locked and limited conversation to collaborators Aug 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants