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

Fix unhandled case of empty cs buckets response #144

Merged
merged 1 commit into from Dec 18, 2013

Conversation

engelsanchez
Copy link
Contributor

the PB translation generates a single token instead of a default record
when the entire message has only default values. This was uncovered in
1.4.4, where the cs buckets query was broken.

the PB translation generates a single token instead of a default record
when the entire message has only default values. This was uncovered in
1.4.4, where the cs buckets query was broken.
@seancribbs
Copy link
Contributor

Is there a related riak_test (or giddyup failure) where this is exercised?

@ghost ghost assigned seancribbs Dec 18, 2013
@engelsanchez
Copy link
Contributor Author

There is a specific commit in the 1.4 history for Riak where the query is broken and you hit this error. I'll post it here in a bit.

@engelsanchez
Copy link
Contributor Author

It's basho/riak_kv@0503431. Basically get riak 1.4.2 tags and that, and you'll hit this as the query returns nothing during the test and there's a function clause error there.

@seancribbs
Copy link
Contributor

@engelsanchez Thanks. Honestly we should probably (for 2.0?) fix that endpoint. It shouldn't be sending empty results to the client.

@engelsanchez
Copy link
Contributor Author

I'm not sure what you mean. It is fixed in 1.4.4. That commit is from a point in time when it was broken. This was found while I was debugging and fixing the issue.

@seancribbs
Copy link
Contributor

@engelsanchez I mean the PB service that replies to this request, does it not emit empty (non-done) responses?

@engelsanchez
Copy link
Contributor Author

Nope. It's been fixed. I didn't look at the semantics of the request though. If you think now that it's fixed this is moot, fine. But having it just crash on a function_clause seemed a bit harsh at the time.

@seancribbs
Copy link
Contributor

Ok. The impact of this change is minimal, and some people are liable to be running a broken version, so it's worth fixing. Thanks for bearing with me.

👍 💃 🍻

@engelsanchez
Copy link
Contributor Author

Thanks for looking at this!

engelsanchez added a commit that referenced this pull request Dec 18, 2013
Fix unhandled case of empty cs buckets response
@engelsanchez engelsanchez merged commit 8d33c02 into 1.4 Dec 18, 2013
@engelsanchez engelsanchez deleted the bugfix/empty-cs-buckets-error branch December 18, 2013 22:29
@seancribbs seancribbs removed their assignment May 8, 2015
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