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

Don't strip away Consul metadata if we return a single value #92

Closed
wants to merge 1 commit into from
Closed

Don't strip away Consul metadata if we return a single value #92

wants to merge 1 commit into from

Conversation

nickpegg
Copy link
Contributor

Fixes an inconsistency outlined in #39 where a KV get
call returns an array if >1 value is returned, but if one value is returned it
strips away the metadata and hands back the raw value.

While this is a friendly thing to do in some cases, it creates a very annoying
inconsistency when doing a recursive get which leaves out the metadata
(most importantly the key of the returned value) as well as requiring the
consumer to check whether the return value of get is a String or Array.

Fixes an inconsistency outlined in #39 where a KV `get`
call returns an array if >1 value is returned, but if one value is returned it
strips away the metadata and hands back the raw value.

While this is a friendly thing to do in some cases, it creates a very annoying
inconsistency when doing a recursive `get` which leaves out the metadata
(most importantly the key of the returned value) as well as requiring the
consumer to check whether the return value of `get` is a String or Array.
@evan2645
Copy link

👍 we need this bad. we'll have to run a fork in the meantime

@EugenMayer
Copy link
Contributor

dupe of #51 AFAICS but i guess it is nonsense creating pull requests here anyway

@nickpegg
Copy link
Contributor Author

Closing in favor for #51

@nickpegg nickpegg closed this Jul 20, 2016
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