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

Mark item done after validity check #274

Merged
merged 2 commits into from
May 14, 2018

Conversation

asymmetric
Copy link
Contributor

@asymmetric asymmetric commented May 14, 2018

This PR improves two things about the way in which we handle the queue:

  • Only mark items as done after we've checked them for validity (i.e. checked that they are strings)
  • Call Forget for invalid items, so that we don't get them in the queue again.

May or may not close #122, not sure because I haven't been seeing that error in a while.

Closes #222.

Lorenzo Manacorda added 2 commits May 14, 2018 14:21
If an item is not a string, then it's invalid, and there's no point in
requeuing it since it will still be invalid.

Signed-off-by: Lorenzo Manacorda <lorenzo@kinvolk.io>
By moving the call to `defer` below, we make sure that we don't mark
invalid items as `Done`, since the `defer` is now below the check for
validity.

Signed-off-by: Lorenzo Manacorda <lorenzo@kinvolk.io>
@iaguis
Copy link
Contributor

iaguis commented May 14, 2018

#122 is already closed?

Copy link
Contributor

@iaguis iaguis left a comment

Choose a reason for hiding this comment

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

lgtm in any case

@asymmetric
Copy link
Contributor Author

Updated, was the wrong issue, but also I'm not sure there's a connection.

@asymmetric asymmetric merged commit c63aad6 into habitat-sh:master May 14, 2018
@asymmetric asymmetric deleted the asymmetric/mark-done branch May 14, 2018 13:22
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.

Mark key as done only if it is Errors when deleting Habitat resource
2 participants