Publish method does not return "confirms.Published" counter(deliveryTag)

[kamal-github]

I have a listener registered through NotifyPublish and it is asynchronously getting confirmation and if ack=false then I republish the message or act upon it, but I get delivery tag from this listener channel and I do not know which payload this delivery tag is of.

While looks like we can return the https://github.com/streadway/amqp/blob/v1.0.0/confirms.go#L36 which looks like the delivery tag for the given channel, but we are ignoring it in Publish method https://github.com/streadway/amqp/blob/v1.0.0/channel.go#L1360.

[chrisDeFouRire]

Hi @kamal-github

I was wondering the same, until I remembered that confirmations are sent on the channel where I publish, only for what I publish on that channel...

ie. confirmation notification with DeliveryTag=1 is for the first message you published on the channel, etc... It's easy to keep track of it (but I agree, Publish could have returned that counter).

[kamal-github]

@chrisDeFouRire I am worried about maintaing the same counter which might have concurency issue. How about adding a method like Java client has. getNextSequenceNumber() ? https://www.rabbitmq.com/tutorials/tutorial-seven-java.html

[chrisDeFouRire]

@kamal-github I like your solution, though I can only give it my vote 👍

[michaelklishin]

This client uses a really conservative approach when it comes to even theoretically breaking public API changes. Exposing a sequence counter reader — is the way to go.

[michaelklishin]

Please submit a PR that introduces a new function that returns the counter, or a doc update if it already exists. Breaking Channel.Publish modifications (such as the return value) are out of the question for this client.

[chrisDeFouRire]

@michaelklishin I think that's exactly what PR #478 (by kamal-github) does

[kamal-github]

@michaelklishin As @chrisDeFouRire mentioned that #478 I have already created the PR for this. Please review. :)

[awa12007]

@michaelklishin

When merging it. #478

[awa12007]

@michaelklishin As @chrisDeFouRire mentioned that #478 I have already created the PR for this. Please review. :)

When merging it

[samyonr]

For everyone who reaches this thread now, note that it was fixed in #478 (but not released yet) and the exact same commit was released in rabbitmq/amqp091-go: https://github.com/rabbitmq/amqp091-go/releases/tag/v1.2.0