Skip to content
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

KV ResumeAtRevision option for watches #478

Closed
mtmk opened this issue Apr 16, 2024 · 3 comments · Fixed by #491
Closed

KV ResumeAtRevision option for watches #478

mtmk opened this issue Apr 16, 2024 · 3 comments · Fixed by #491
Assignees
Labels
good first issue Good for newcomers help wanted Extra attention is needed new-feature

Comments

@mtmk
Copy link
Collaborator

mtmk commented Apr 16, 2024

@mtmk mtmk added good first issue Good for newcomers help wanted Extra attention is needed new-feature labels Apr 16, 2024
@niklasfp
Copy link
Contributor

@mtmk as per slack conversation, I'll like to give this one a shot.

Initial idea:

NatsKvOpts.cs
Would add ResumeAtRevision int prop to NatsKVWatchOpts (document it to be "The revision to start from, if set to 0 (default) this will be ignored"
https://github.com/nats-io/nats.net.v2/blob/dcfaac12777ba1d6e873e34199b3595f816319e0/src/NATS.Client.KeyValueStore/NatsKVOpts.cs#L43

NatsKVWatcher.cs
It's using the _sequenceStream here https://github.com/nats-io/nats.net.v2/blob/dcfaac12777ba1d6e873e34199b3595f816319e0/src/NATS.Client.KeyValueStore/Internal/NatsKVWatcher.cs#L353
And I was thinking about setting _sequenceStream = opts.ResumeAtRevision here https://github.com/nats-io/nats.net.v2/blob/main/src/NATS.Client.KeyValueStore/Internal/NatsKVWatcher.cs#L78

@mtmk
Copy link
Collaborator Author

mtmk commented Apr 24, 2024

@niklasfp looking at this a bit closer I think it's good. Setting the _sequenceStream = opts.ResumeAtRevision in NatsKVWatcher.ctor should work since it will treacle down to the consumer create request down below and also should be updated as expected on reconnects. feel free to open a PR when you're ready and have time. thank you 💯

@niklasfp
Copy link
Contributor

@niklasfp looking at this a bit closer I think it's good. Setting the _sequenceStream = opts.ResumeAtRevision in NatsKVWatcher.ctor should work since it will treacle down to the consumer create request down below and also should be updated as expected on reconnects. feel free to open a PR when you're ready and have time. thank you 💯

For future reference and history, setting the sequence in the ctor was not an option because of the way watchers gets re-created.

@mtmk mtmk closed this as completed in #491 Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed new-feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants