Roblox / rs-consul

This crate provides access to a set of strongly typed apis to interact with consul (https://www.consul.io/)
MIT License
35 stars 23 forks source link

Properly handle check and set when it is set to 0 #26

Closed Szymongib closed 1 year ago

Szymongib commented 2 years ago

What problem are we solving?

When check_and_set is set to 0, the query param is omitted completely which is valid for use case when we want to update regardless of current modify_index. This however make it impossible to execute requests with check_and_set set to 0, which is also valid use case for setting key only if it does not already exists.

This is even stated in the comment however it does not reflect the actual behaviour because the cas query param is skipped when the value is 0.

How are we solving the problem?

Make check_and_set an Option<i64> to clearly distinguish between 0 and absence of value.

Important note

This is technically breaking change as programs using current version of the library will not compile, therefore it should be included in the next major version.

Checks

Please check these off before promoting the pull request to non-draft status.

(Not sure where can I sign CLA. Build will not run automatically for first time contributors.)

arthurprs commented 1 year ago

This is an important "fix", as it's necessary to get correct CAS semantics.

github-actions[bot] commented 1 year ago

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

Szymongib commented 1 year ago

I have read the CLA Document and I hereby sign the CLA