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

HTTP client API all-in-one [3/3] #1398

Merged
merged 7 commits into from
Apr 19, 2016

Conversation

hmmr
Copy link
Contributor

@hmmr hmmr commented Apr 17, 2016

This is Part 3/3 of #1382 broken up and reassembled into new commits.

Requires (includes) #1396, #1397 (with basho/riak_pb#190 (RIAK-2492)).

A two month worth of commits by @hmmr and @lehoff, squashed.

The odd change in riak_kv_qry_worker (preserving lists for records in the return value of query) goes with the list_to_tuple conversion now happening in riak_kv_ts_api. Native lists from riak_kv_qry:submit are immediately consumable by mochijson2:encode, and this is the form we would like to have the query results in wm_timeseries_query.

A two month worth of commits by @hmmr and @lehoff, squashed.

The odd change in riak_kv_qry_worker (preserving lists for records in
the return value of query) goes with the list_to_tuple conversion now
happening in riak_kv_ts_api. Native lists from riak_kv_qry:submit are
immediately consumable by mochijson2:encode, and this is the form we
would like to have the query results in wm_timeseries_query.
@hmmr hmmr force-pushed the feature-az+th-http_client_api_allinone branch from 3948210 to bf62ab8 Compare April 19, 2016 11:27
@@ -292,10 +292,8 @@ prepare_final_results2(#riak_sel_clause_v1{col_return_types = ColTypes,

prepare_final_results_test() ->
Rows = [[12, <<"windy">>], [13, <<"windy">>]],
Copy link

Choose a reason for hiding this comment

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

Shouldn't this be tuples then? I'll test and find out...

Copy link
Contributor Author

@hmmr hmmr Apr 19, 2016

Choose a reason for hiding this comment

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

list_to_tuple now happens in riak_kv_ts_svc, right before the records are delivered to the PB/TTB code and further to the client. This is so in order to avoid converting tuples back to lists in riak_kv_wm_timeseries_query, where lists are required for converting the data to json.

Which is why we still have native records-as-lists here.

Brett Hazen and others added 3 commits April 19, 2016 16:40
A two month worth of commits by @hmmr and @lehoff, squashed.

The odd change in riak_kv_qry_worker (preserving lists for records in
the return value of query) goes with the list_to_tuple conversion now
happening in riak_kv_ts_api. Native lists from riak_kv_qry:submit are
immediately consumable by mochijson2:encode, and this is the form we
would like to have the query results in wm_timeseries_query.
@hmmr hmmr force-pushed the feature-az+th-http_client_api_allinone branch from bf62ab8 to 1912a98 Compare April 19, 2016 20:53
{ReqId, {error, timeout}} ->
{mochijson2:encode({struct, [{error, timeout}]}), done};
_Weird ->
%% @todo: should we log this?
Copy link

Choose a reason for hiding this comment

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

Probably a good idea. Might be useful to see if bad requests are being sent.

@hazen
Copy link

hazen commented Apr 19, 2016

👍 94db162

@hazen
Copy link

hazen commented Apr 19, 2016

@borshop merge

@hazen hazen merged commit 78374b5 into riak_ts-develop Apr 19, 2016
@hazen hazen deleted the feature-az+th-http_client_api_allinone branch April 19, 2016 23:13
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.

2 participants