-
Notifications
You must be signed in to change notification settings - Fork 34
feat(js): add support for subject-based sequence constraints #287
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but comments are a bit weird.
jetstream/tests/jetstream_test.ts
Outdated
| const js = jsm.jetstream(); | ||
|
|
||
| await Promise.all([ | ||
| js.publish("a.1.foo", "1:1"), // Last is 1 for aa.1.foo; 1 for a.1.*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo in comment aa.1.foo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
jetstream/tests/jetstream_test.ts
Outdated
| await pc("a.1.bar", "a.1.*", 3, false); | ||
| await pc("a.1.bar", "a.1.*", 4, false); | ||
| await pc("a.1.bar", "a.1.*", 5, false); | ||
| await pc("a.1.bar", "a.1.*", 6, true); // Last is 8 for a.1.bar; 8 for a.1.*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand that you copied the comment from the Go client, but I think it is wrong. Last is 6 for a.1.bar. However, inserting that new message will make a.1.bar sequence 8, since the original publishes ended at 7.
Maybe that's what the comment means, but if anything, the comment should explain why "6" is the proper sequence for this subject.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes I c/p the ns test... Let me print the sequences that it onboards and update the comment with that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed the comments/doc to be more useful
Introduced the `lastSubjectSequenceSubject` option to enable subject-specific sequence constraints when publishing messages. Updated relevant headers, types, and tests to support this feature. See nats-io/nats-server#5280 Signed-off-by: Alberto Ricart <[email protected]>
Signed-off-by: Alberto Ricart <[email protected]>
Introduced the
lastSubjectSequenceSubjectoption to enable subject-specific sequence constraints when publishing messages. Updated relevant headers, types, and tests to support this feature.See nats-io/nats-server#5280