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

Fully unit tested consul-powered storage #16

Merged
merged 1 commit into from Sep 18, 2015

Conversation

lord2800
Copy link
Contributor

Depends on #1 which is now ready to be merged.

@lord2800
Copy link
Contributor Author

Important note: add and update are not atomic operations due to hashicorp/consul#886.

function ConsulBackend(options) {
options = options || {};
this._prefix = options.prefix || 'billimanjaro';
delete options.prefix;

Choose a reason for hiding this comment

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

Why are you deleting the the array prefix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I can send the remaining options, without the prefix (just in case for backwards compatibility), to the consul client.

@gabelimon
Copy link

👍 all of my comments seem minor.

@lord2800
Copy link
Contributor Author

Wanna give it another once-over @gabelimon? I cleaned up a few minor things and moved the add/update common validation logic into its own function (with proper error types, even).

@gabelimon
Copy link

👍

lord2800 added a commit that referenced this pull request Sep 18, 2015
Fully unit tested consul-powered storage
@lord2800 lord2800 merged commit dda7fac into billimanjaro:master Sep 18, 2015
@lord2800 lord2800 deleted the feat/consul-backend branch September 18, 2015 02:08
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.

None yet

2 participants