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

Add Natjs KeyValue Store #132

Merged
merged 2 commits into from
Dec 26, 2023
Merged

Add Natjs KeyValue Store #132

merged 2 commits into from
Dec 26, 2023

Conversation

kobergj
Copy link
Contributor

@kobergj kobergj commented Dec 1, 2023

Adds a store nats-js-kv based on nats-js KeyValue interface. Instead of using streams to store the data (like the nats-js store does) it will store the data in jetstreams key value stores.

Some implementation details:

  • optionally, keys can be base32 encoded to work around limited characters usable as keys for the store
  • List calls can take quite long when there are many keys in a bucket. Might need a follow-up to fix
  • I copied most tests from nats-js package to have the same test coverage. We should think about running the same testsuite against all stores.

I also needed to edit the .golangci.yml to make it possible getting green pipelines:

  • The configuration for the depguard linter had a wrong format

@kobergj kobergj force-pushed the NatjsKVStore branch 11 times, most recently from 65b63aa to 4d424e3 Compare December 7, 2023 14:32
@kobergj kobergj marked this pull request as ready for review December 7, 2023 14:40
@kobergj kobergj changed the title WIP: Add Natjs KeyValue Store Add Natjs KeyValue Store Dec 7, 2023
@jochumdev
Copy link
Contributor

While I like the base32 encoding here for simplicity, I believe this should be optional. Yeah, can be by default on.

Why:

So what do you think?

@kobergj
Copy link
Contributor Author

kobergj commented Dec 12, 2023

Yes. Good point. 👍 I introduced EncodeKeys option. Default is false because this is what I would expect.

Signed-off-by: jkoberg <jkoberg@owncloud.com>
Signed-off-by: jkoberg <jkoberg@owncloud.com>
@kobergj
Copy link
Contributor Author

kobergj commented Dec 15, 2023

@jochumdev since this a new store and not just some fix or enhancement, I would very much appreciate your approve before I merge it

@asim asim merged commit 94a49ba into micro:main Dec 26, 2023
3 checks passed
@kobergj kobergj deleted the NatjsKVStore branch December 27, 2023 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants