[CHANGED] JetStream: accept "Nats-Expected-Last-Sequence" with "0" #3038
Conversation
|
Looks good, but seems unlikely @derekcollison would have forgotten to add this when we did the per-subject one to support KV Create(), so maybe there was a reason for not doing it? Do you remember anything @derekcollison ? |
|
@ripienaar That's what I fear too, so a bit hesitant and would not mind if we don't add this. In the client PR you mentioned that we should have it for consistency (compared to the last-ser-per-subject). Have you changed your mind seeing the code change? |
|
I reckon it has good value both in being consistent and as a feature - but I am also aware we are very near a big release and might punt till 2.9.x, so would be fine if we waited a tad to give it proper thought. |
|
@ripienaar Could not agree more. Will switch this PR to draft and will reconsider post 2.8.0. |
|
@derekcollison Ah yes. There may be a use-case, but yeah, pretty limited. Let's leave this as draft for now. |
|
@bruth @ripienaar Given the limitation described by Derek here: would you still consider that there is value to this PR? If so, I will move it out of Draft... |
|
I do think it would be for consistency in behavior.. obviously a call-out of this one-time situation in the docs would be important, but I am still in favor. |
We use to ignore if the seq was 0, but now would treat it as a requirement that the stream be empty if header is present but set to 0. This relates to client PR: nats-io/nats.go#958 Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
kozlovic
force-pushed
the
js_expected_last_seq_zero
branch
from
43b2641
to
be8734e
Compare
|
@derekcollison @bruth @ripienaar I have rebased from main branch and switched from Draft to ready for review. If approved I will merge. |
We use to ignore if the seq was 0, but now would treat it as
a requirement that the stream be empty if header is present but
set to 0.
This relates to client PR: nats-io/nats.go#958
Signed-off-by: Ivan Kozlovic ivan@synadia.com