Change ConsumerConfig AckPolicy default to Explicit

[hanlong-chen-1047]

Currently the no-args constructor for ConsumerConfig does not set the AckPolicy for the consumer, which defaults it to None. This is confusing given that the NATS docs state that the default AckPolicy is Explicit: https://docs.nats.io/nats-concepts/jetstream/consumers#ackpolicy

This has led to unexpected behaviors since we had expected this to be set to Explicit by default for all consumers.

This PR changes the no-args ConsumerConfig constructor to set the AckPolicy to Explicit, to be the same as the constructor with a durable name.

[mtmk]

This sounds like a sensible thing to do actually (potentially related discussions)

cc @robertmircea

[caleblloyd]

Should we consider changing the default value of the enum ConsumerConfigAckPolicy to Explicit instead of None?

[mtmk]

Should we consider changing the default value of the enum ConsumerConfigAckPolicy to Explicit instead of None?

it might make more sense for other scenarios as well e.g.

    AckPolicy = default,

Edit: @hanlong-chen-1047 are you also happy with this suggestion? (I can make the change if you don't have time, not to worry)

[robertmircea]

Fine with me. I was setting it to Explicit every time

On Wed, Apr 24, 2024 at 22:26 Ziya Suzen @.***> wrote:

This sounds like a sensible thing to do actually (potentially related discussions https://github.com/nats-io/nats.net.v2/discussions?discussions_q=consumerconfig )

cc @robertmircea https://github.com/robertmircea

— Reply to this email directly, view it on GitHub https://github.com/nats-io/nats.net.v2/pull/490#issuecomment-2075782699, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA5V65665RKTLUCWQR4GATY7AIN7AVCNFSM6AAAAABGXLJOIKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZVG44DENRZHE . You are receiving this because you were mentioned.Message ID: @.***>

[mtmk]

looking at this closer enum:

public enum ConsumerConfigAckPolicy
{
    Explicit = 0, // <--- hurts my eyes
    All = 1,
    None = 2,
}

...having None = 0 is more conventional and it's aligned with how the server is working on the wire.

[hanlong-chen-1047]

Should we consider changing the default value of the enum ConsumerConfigAckPolicy to Explicit instead of None?

it might make more sense for other scenarios as well e.g.

    AckPolicy = default,

Edit: @hanlong-chen-1047 are you also happy with this suggestion? (I can make the change if you don't have time, not to worry)

yeah that'd be great

[caleblloyd]

There are some usages / examples now that no longer need to explicitly set the Ack policy to Explicit

https://github.com/nats-io/nats.net.v2/blob/f6c6419f34741dc9dabfbe400c4861d25901a579/tests/NATS.Client.JetStream.Tests/JetStreamTest.cs#L81

https://github.com/nats-io/nats.net.v2/blob/f6c6419f34741dc9dabfbe400c4861d25901a579/tests/NATS.Client.CheckNativeAot/Program.cs#L91

https://github.com/nats-io/nats.net.v2/blob/f6c6419f34741dc9dabfbe400c4861d25901a579/tests/NATS.Client.Core.MemoryTests/NatsConsumeTests.cs#L32

https://github.com/nats-io/nats.net.v2/blob/f6c6419f34741dc9dabfbe400c4861d25901a579/tests/NATS.Net.DocsExamples/JetStream/ManagingPage.cs#L73

https://github.com/nats-io/nats.net.v2/blob/f6c6419f34741dc9dabfbe400c4861d25901a579/src/NATS.Client.JetStream/Models/ConsumerConfig.cs#L32