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

Resolve inconsistent recursive return for Diplomat::KV.get when singl… #120

Merged
merged 2 commits into from Aug 2, 2017

Conversation

taharah
Copy link
Member

@taharah taharah commented Jan 11, 2017

…e value return

Resolves #119

@t3hk0d3
Copy link

t3hk0d3 commented Jan 11, 2017

Not sure this is backward-compatible. I think it would break interface of non-recursive get.

@taharah
Copy link
Member Author

taharah commented Jan 11, 2017

Yea I was thinking the same thing since it broke the spec tests and I would do a major version bump if/when this got merged. I am hoping to get the gem up to speed with the current API endpoints in Consul 0.7.2 ASAP before cutting a new release.

@taharah taharah added this to the 2.0.0 milestone Jan 11, 2017
@taharah taharah added this to Fix breaking issues in Version 3.0.0 Apr 15, 2017
@EugenMayer
Copy link
Contributor

This is so much needed, i would really love to see this getting fixed - major version bump is reasonable. Anything i can do to move this forward? Right now, i create dummy values in consul to avoid this bug... which is crazy

@t3hk0d3
Copy link

t3hk0d3 commented Jul 21, 2017

@EugenMayer Well, quick workaround is to parse KV data yourself.

@taharah
Copy link
Member Author

taharah commented Aug 2, 2017

@EugenMayer I just saw there was a major version bump in the past week, so I'm going to get this staged up and pushed out as 2.0.1 since I know this is really needed. In the next month or so, I plan to put some real TLC into this gem as I'll need it for an upcoming project.

@taharah
Copy link
Member Author

taharah commented Aug 2, 2017

@EugenMayer I've rebased the changes onto the latest master branch, so if you could give this a quick +1 I'll move forward with getting this merged and released.

@EugenMayer
Copy link
Contributor

i am so happy to have this one in, totally +1

@taharah taharah merged commit e4b8878 into WeAreFarmGeek:master Aug 2, 2017
@taharah
Copy link
Member Author

taharah commented Aug 2, 2017

@EugenMayer v2.0.1 has been tagged and released. enjoy!

@EugenMayer
Copy link
Contributor

well this more or less has broken all diplomat integrations and i think the reason is, it does not play nice with "convert_to_hash" .. this is a huge issue.

@EugenMayer
Copy link
Contributor

created #152

@johnhamelink
Copy link
Member

@EugenMayer could you provide more detail please? Are you comparing all diplomat integrations for the previous major revision, of do you mean the 2.0.1 release compared with 2.0.0?

@johnhamelink
Copy link
Member

Thanks the issue answers my question

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Version 3.0.0
Fix breaking issues
Development

Successfully merging this pull request may close these issues.

None yet

4 participants