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

Issue1155 AMQP ack refactor #1174

Merged
merged 3 commits into from
Aug 19, 2024
Merged

Issue1155 AMQP ack refactor #1174

merged 3 commits into from
Aug 19, 2024

Conversation

reidsunderland
Copy link
Member

AMQP: refuse to attempt to ack a message if it was received on a different channel, connection or broker (preventing unnecessary
connection tear down and re-establish)

AMQP: when ack is attempted and fails because the connection to the broker is broken, cleanup (close) the broken connection and don't bother attempting to retry the ack

Also changed the ack method to return a boolean, which wasn't needed but could possibly be useful someday if we want to change how we handle a failed ack.

  failures to the caller (message.gather's ack method)
- message.gather's ack now deletes the ack_id from messages instead of
  the moth classes
- AMQP: refuses to attempt to ack a message if it was received on a
  different channel, connection or broker (preventing unecessary
connection tear down and re-establish)
- AMQP: when ack is attempted and fails because the connection to the
  broker is broken, don't loop and retry. After re-connecting, acking
that message would always fail, prevents a potential infinite loop.
…ith the ack_id after a failed ack is protocol specific
Copy link

github-actions bot commented Aug 19, 2024

Test Results

1 tests   0 ✅  14s ⏱️
1 suites  0 💤
1 files    0 ❌  1 🔥

For more details on these errors, see this check.

Results for commit 2891472.

♻️ This comment has been updated with latest results.

@reidsunderland reidsunderland changed the title Issue1155 ack refactor Issue1155 AMQP ack refactor Aug 19, 2024
@petersilva
Copy link
Contributor

The tests are messed up atm... but I think this change is ok anyways... in an ideal world we would fix the tests... but no clue what is with 20.04.

@petersilva petersilva merged commit 4148111 into development Aug 19, 2024
46 of 93 checks passed
@reidsunderland reidsunderland deleted the issue1155_ack_refactor branch November 20, 2024 18:28
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