Skip to content

Actually nack when using 'Nack message requeue true' #9715

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 17, 2023
Merged

Actually nack when using 'Nack message requeue true' #9715

merged 1 commit into from
Oct 17, 2023

Conversation

kjnilsson
Copy link
Contributor

Option in mgmt UI.

Gsantomaggio
Gsantomaggio approved these changes Oct 17, 2023
View reviewed changes
Copy link
Member

@Gsantomaggio Gsantomaggio left a comment

Choose a reason for hiding this comment

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

LGTM

I think that the atom ack_requeue_true is not correct here; maybe it would be better nack_requeue_true. BTW I would not change it to avoid losing compatibility with external tools like rabbitmqadmin get queue=my_quorum_q ackmode=ack_requeue_true

@michaelklishin
Copy link
Collaborator

@Gsantomaggio note that we cannot easily change the atoms as it would change the API. Maybe for 3.13 but not in this PR which we will backport.

@michaelklishin michaelklishin merged commit 368f3ef into main Oct 17, 2023
@michaelklishin michaelklishin deleted the mgmt-get-msg-nack branch October 17, 2023 14:48
@mergify mergify bot mentioned this pull request Oct 17, 2023
Merged
@Gsantomaggio
Copy link
Member

@michaelklishin yes, I agree.

michaelklishin added a commit that referenced this pull request Oct 17, 2023
@michaelklishin
Merge pull request #9720 from rabbitmq/mergify/bp/v3.12.x/pr-9715
5e6c5a2 
Actually nack when using 'Nack message requeue true' (backport #9715)
michaelklishin added a commit that referenced this pull request Oct 17, 2023
@michaelklishin
Merge pull request #9721 from rabbitmq/mergify/bp/v3.11.x/pr-9720
a01b6a7 
Actually nack when using 'Nack message requeue true' (backport #9715) (backport #9720)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Gsantomaggio Gsantomaggio Gsantomaggio approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kjnilsson @michaelklishin @Gsantomaggio