PB codec for get_coverage [2/3] #1397
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 3 commits
April 13, 2016 00:50
* rows as tuples, not lists of cells; * [] for undefined.
This was referenced Apr 17, 2016
hmmr
force-pushed
the
reorg-az-pb_codec_for_get_coverage
branch
from
dcad486
to
2bb5e6a
Compare
added 2 commits
April 19, 2016 13:55
relies on the code moved from riak_kv_ts_svc over to riak_pb_ts_codec.
hmmr
force-pushed
the
reorg-az-pb_codec_for_get_coverage
branch
from
2bb5e6a
to
3cb7eb1
Compare
| Bucket = riak_kv_ts_util:table_to_bucket(Table), | ||
| Client = {riak_client, [node(), undefined]}, | ||
| convert_cover_list(sql_to_cover(Client, Queries, Bucket, []), State) | ||
| ?E_BAD_QUERY, flat_format("Failed to parse query: ~p", [Reason])), |
There was a problem hiding this comment.
Using ~p means this message will always look really bad, if it is a string it will speech marks it will have all the bracks. If I force a parsing error in riak_test I get this.
================ ts_cluster_coverage failure stack trace =====================
{{badmatch,{error,{1018,
<<"Failed to parse query: {<<\"selectah\">>,riak_ql_parser,\n [\"syntax error before: \",\"identifier"]}">>}}},
[{ts_cluster_coverage,test_quanta_range,5,
[{file,"tests/ts_cluster_coverage.erl"},{line,54}]},
{ts_cluster_coverage,confirm,0,
[{file,"tests/ts_cluster_coverage.erl"},{line,45}]},
{riak_test_runner,return_to_exit,3,
[{file,"src/riak_test_runner.erl"},{line,196}]}]}
==============================================================================
There was a problem hiding this comment.
A proper error retval matching is in order. I did this for the main tsqueryreq case; time to fix it here, too.
|
+1 2ad8f53 |
borshop
added a commit
that referenced
this pull request
Apr 19, 2016
PB codec for get_coverage [2/3] Reviewed-by: andytill
|
@borshop merge |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is Part 2/3 of #1382 broken up and reassembled into new commits.
Requires (includes) #1396 and basho/riak_pb#190.
This PR removes the code assembling
#tscoveragerespfrom riak_kv, to be added in the linked riak_pb PR. The callbacks in the common module serving both PB and TTB messages need to only ever return plain Erlang terms. For PB messages, appropriate codecs will be used (from riak_pb_ts_codec), while TTB messages are transmitted (as if) without any conversion.