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

Cv2 4606 delete message #85

Merged
merged 2 commits into from
May 22, 2024
Merged

Cv2 4606 delete message #85

merged 2 commits into from
May 22, 2024

Conversation

DGaffney
Copy link
Collaborator

If we don't delete the existing message this code just has the net effect of compounding the same task repeatedly which is definitely not what we want! Increment the count, delete the old job, and add a new job.

Copy link
Contributor

@skyemeedan skyemeedan left a comment

Choose a reason for hiding this comment

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

lol, this makes sense. Does it work locally?

@@ -144,4 +144,5 @@ def increment_message_error_counts(self, messages_with_queues: List[Tuple]):
else:
updated_message = schemas.parse_message(message_body)
updated_message.retry_count = retry_count
queue.delete_messages(Entries=[self.delete_message_entry(message)])
Copy link
Contributor

Choose a reason for hiding this comment

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

this isn't including the idx (which is mapped to the id) is it needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yep - idx is only needed as some unique identifier in a list of items - and on the other side of this, it just defaults to 0 if not passed along, so this works fine!

@DGaffney
Copy link
Collaborator Author

lol, this makes sense. Does it work locally?

lol, yep - and yes I did test this on the QA box directly to make sure that message deletion would work, as I was figuring out what the error was in this latest deploy

@DGaffney DGaffney requested a review from skyemeedan May 22, 2024 12:23
@DGaffney DGaffney merged commit 7869707 into master May 22, 2024
2 checks passed
@DGaffney DGaffney deleted the cv2-4606-delete-message branch May 22, 2024 12:41
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.

2 participants