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

Add HTTP API for datatypes. #732

Merged
merged 3 commits into from
Nov 25, 2013
Merged

Add HTTP API for datatypes. #732

merged 3 commits into from
Nov 25, 2013

Conversation

seancribbs
Copy link
Contributor

This rewrites riak_kv_wm_crdt and ensures that the functionality it exposes is similar to PB. The JSON format of request/response payloads is described in detail at the top of the module, and implemented in riak_kv_crdt_json.

Ctx#ctx{client=Client,
bucket=path_segment_to_bin(bucket, RD),
key=path_segment_to_bin(key, RD),
crdt_type=proplists:get_value(crdt, wrq:path_info(RD)),
Copy link
Member

Choose a reason for hiding this comment

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

I thought this came from the bucket_type now (that the path segment was just the string "datatypes")? Should this still be in the code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, I missed this.

* Remove extraction of undefined path segment into the resource
  context.
* Guard the timeout parameter check to make sure it is greater than 0.
* Add explanatory comment about the redirection when using the default
  bucket-type.
{Result, RD, Ctx};
TimeoutStr when is_list(TimeoutStr) ->
try list_to_integer(TimeoutStr) of
Timeout when is_integer(Timeout), Timeout > 0 ->
Copy link
Member

Choose a reason for hiding this comment

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

Did you figure out if > 0 is correct? I wonder if >= 0 is better, as you say, then 0 can mean infinity?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think in the other APIs, yes 0 means infinity, but this doesn't coerce it. Fixing.

@russelldb
Copy link
Member

Ran eunit tests and tested by hand using riak-erlang-http-client#28.
Looks good, works for me. As mentioned, a riak_test would be nice. We can probably generalise the existing test that uses the PB client. 👍

seancribbs pushed a commit that referenced this pull request Nov 25, 2013
@seancribbs seancribbs merged commit 2bba0e8 into develop Nov 25, 2013
@seancribbs seancribbs deleted the feature/sdc/crdt-http-api branch November 25, 2013 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants