Improve StreamConfig ergonomics

[glueball]

Use Case:

Configure new JS streams in an easier way. In my concrete use case, I have a microservice architecture, where every service creates/updates the streams it manages (ie. the streams it publishes to, or that it listens as a work queue). At service startup, the service checks if all the streams exists, or if they need to be updated.

Currently, there are some confusing options. For instance, what is the correct way of setting no limit in max_bytes or max_msgs? Is it 0? Is it -1? Even worse, some options use different strategies: max_msg_size is an Option<i32>, is it None the same as Some(-1)? Finally, for no_ack is None different from Some(false)?

I had to check the stream using the nats cli to check that I was configuring the stream correctly (which also has some gotchas, as the maximum age displays "0.00s" instead of the string "unlimited").

Proposed Change:

Use enums to be more expressive. Create a simple method to generate the underlying value needed by JetStream.

For file size options (max_bytes and max_msg_size), something like that

enum SizeLimit
{
    Unlimited,
    Bytes(u64),
   // Maybe also options for kb, mb, etc.
}    

For time based options (max_age, duplicate_window)

enum TimeLimit
{
    Unlimited,
    Seconds(u64),
    Minutes(u64),
    Hours(u64),
    Days(u64),
    Months(u64),
    Years(u64),
}

Also, subjects doesn't need to be an Option. A vec![] would mean the same.

Other enums are possible for other options, but those are the ones I found more confusing.

Finally, all those enums and StreamConfig itself should implement PartialEq and Eq, making it easier to check if a Stream needs to be updated.

Who Benefits From The Change(s)?

I'd say that all users of the Rust client could benefit of this change, as it would make the configuration much easier to understand. Of course, this would be a breaking change, but if you guys consider this or a similar approach, doing so before 1.0.0 is the appropriate time.

Alternative Approaches

Improve the documentation. For every option, include the meaning of every special value (i.e, how to set a "no limits"). In some cases, it should specify the units (bytes, seconds, etc).

Another approach would be to create a StreamConfigBuilder type, where there could be methods like set_max_bytes_unlimited() and set_max_bytes(max: u64). This would give similar ergonomic improvement without the BC break. However, the generated StreamConfig would still be a bit confusing itself.

[spacejam]

Hi @glueball, I share all of your perceptions. Some of this complexity is reducible, and I've taken a stab at simplifying some things in #197. There are a few things that need to match the underlying JSON configuration object however, which is not as practical to address without adding an expensive and bug-prone-to-maintain translation layer between various configuration objects. I think in general it's better to keep the mapping very thin between the Rust configuration and the underlying JSON configuration that is consumed by a Golang nats-server. Some of the strangeness comes from a mixture of go-isms and c-isms making their way into the JSON, such as the -1-for-default stuff that pops up. However, I've added some serde tricks to avoid a little bit of the option-wrapping in a way that hopefully reduces some of the rust-only cruft, to reduce the overall maintenance burden of the mapping from client to json to server.

[glueball]

Hi @spacejam, sorry I didn't respond earlier. I think I've fixed my notification settings now.

I understand your reasons abut matching the underlying API and maintainability. My approach in similar situations is to implement custom serde serializers and deserializers for every enum, so you can go back and forth the JSON and the Rust struct without need for having different versions of the structs. For instance, you can make serde deserialize "-1" to SizeLimit::Unlimited, and any other value to SizeLimit::Bytes(value).

However, I've already tried 0.13 and the changes there really improve usability. Thanks!

Can I suggest to add in the "rustdoc" for every field what are the "special values"?