8.0 Proposal: Remove WaitForConfirms and PublishBatch methods/APIs

[stebet]

I wanted to bring this proposal forward with the aim to simplify the base client interface, and rather make it easier to extend (later proposals, have some ideas there).

As @lukebakken mentioned here this aims to simplify the basic API, making it minimal in the sense that it simply exposes the AMQP methods and rather provides extensibility points where additional features/logic can be added.

Reasoning: The WaitForConfirms methods are not recommended and streaming confirms is the recommended practice according to the documentation so therefore I don't think it makes sense for them to be provided by the base API, where additional overhead and locking is required to maintain bookkeeping for delivery tags through linked lists and synchronization primitives (CountdownEvent in this case). If the main API provides the required notifications through events or handler interfaces that could easily separate this logic from the main client, making it simpler.

Also, since WaitForConfirms was mostly aimed at being able to wait for batches of messages being confirmed, and since we now have built-in batching (through channels today, or pipelines possibly later), there is really no reason to keep PublishBatch around, since it has been made pretty much redundant. Also, that functionality could also be provided through extensions methods as well rather than be a part of the main client.

[stebet]

Also on that note, if breaking changes are to be made in 7.0.0 do you want that on a separate branch or create a separate 6.X branch for maintenance work having the 7.0.0 release take over the master branch?

[michaelklishin]

There's already a 6.x branch for maintenance. We should bump master to 8.0, create a 7.x off of 6.x and do the smaller changes on 7.x.

[michaelklishin]

Moving to 8.0 so that we can ship a 7.0 with a smaller set of breaking API changes if necessary.

[bollhals]

@michaelklishin Is the batch publish used in other languages or is this C# only?
Also with all this refactorying and improvements, I'm not sure the batch functionality is actually giving much benefit anymore compared to just call BasicPublish. Can we just remove the batch functionality in 7.0?

[bollhals]

ping =)

[michaelklishin]

Batch publish is an invention of this client which (perhaps with some protocol extensions) we would like to one day see in other clients. We can remove the current implementation until then as the only reason for its introduction was certain implementation efficiency gains.

[bo-bac]

have tried mass-publish (many conections with many channels) but with acknowledgment
it seems would be helpful to get connection/session id from sender in order to identifier it 'connectionId-channelNo-seqNo'
now it is 'channelNo-seqNo' and not unique so needs some black magic) code is here