Skip to content
This repository has been archived by the owner on Jun 6, 2024. It is now read-only.

Added support for persistence with KV #65

Merged
merged 15 commits into from
Feb 21, 2019

Conversation

danzlarkin
Copy link
Contributor

@danzlarkin danzlarkin commented Feb 18, 2019

Hey @hankjacobs,

I've added support for persistence via the API for the KeyValueStore.

When it is called with a path passed to the constructor it will create a file using the path specified.

Example usage:

// Read and write from 'example.kvstore.db'
new KeyValueStore('example.kvstore.db')

I've added a test also so you can validate it all works correctly.

I'd also appreciate it if you could release it in a new release on NPM as soon as possible as I'd like to be using this functionality with Restt-CLI.

Keep up the great work :)

Copy link
Contributor

@hankjacobs hankjacobs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this! Stoked on this feature. Could you please also add a flag to the cli that allows the user to specify the path to persist the KeyValueStore? It should be empty by default which should indicate that persistence is off.

lib/kv.js Outdated Show resolved Hide resolved
lib/__tests__/kv.test.js Outdated Show resolved Hide resolved
lib/__tests__/kv.test.js Outdated Show resolved Hide resolved
@danzlarkin
Copy link
Contributor Author

Hey @hankjacobs - added features as requested.

I've added the flag --kv-set a replacement for --set with a deprecation warning logged if --set is used.

I've added the flag --kv-file to use as the file path for KV persistence.

Copy link
Contributor

@hankjacobs hankjacobs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking great! Just some minor changes but overall looks good.

README.md Outdated Show resolved Hide resolved
lib/utils.js Outdated Show resolved Hide resolved
lib/utils.js Outdated Show resolved Hide resolved
@hankjacobs
Copy link
Contributor

Oh and don't forget to update with master

@danzlarkin
Copy link
Contributor Author

@hankjacobs

All patched and optimised now :)

lib/utils.js Outdated Show resolved Hide resolved
lib/utils.js Show resolved Hide resolved
lib/utils.js Outdated Show resolved Hide resolved
@danzlarkin
Copy link
Contributor Author

@hankjacobs

All resolved now - this should be it!

Copy link
Contributor

@hankjacobs hankjacobs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes so close, just one more I think :)

lib/utils.js Outdated Show resolved Hide resolved
@danzlarkin
Copy link
Contributor Author

@hankjacobs

Yes, corrected now!

@hankjacobs hankjacobs merged commit 712dcf6 into dollarshaveclub:master Feb 21, 2019
@hankjacobs
Copy link
Contributor

Merged and beta pushed with it included. I'll play with it tomorrow and publish a new version if it looks good.

@danzlarkin
Copy link
Contributor Author

Excellent - thank you!

@hankjacobs
Copy link
Contributor

@larkin-nz v0.0.11 has been released

@danzlarkin
Copy link
Contributor Author

Excellent - thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants