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 to java w/ version update and update to aae/pb tests for RIAK-1557 i... #459

Merged
merged 2 commits into from
Mar 13, 2015

Conversation

zeeshanlakhani
Copy link
Contributor

...nvolving entropy data and spaces in types/buckets/keys.

This fixes up the issue w/ Entropy Data described in #450 and #436.

@zeeshanlakhani zeeshanlakhani force-pushed the bugfix/zl/fix-for-entropy-data-with-spaces branch from b75e042 to 5c719b3 Compare February 26, 2015 20:21
@zeeshanlakhani
Copy link
Contributor Author

@seancribbs I uploaded yokozuna-2.jar and yokozuna-2.jar.sha to S3.

@@ -9,6 +9,7 @@
-include_lib("basho_bench/include/basho_bench.hrl").
-record(state, {default_field, fruits, pb_conns, index, bucket, iurls, surls}).
-define(DONT_VERIFY, dont_verify).
-define(SPACER, 'foo ').
Copy link
Contributor

Choose a reason for hiding this comment

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

Total nitpick, but mind switching the prefix on space testing keys to something that makes the purpose clear if you're tracking down a problem later?

…7 involving entropy data and spaces in types/buckets/keys
@zeeshanlakhani zeeshanlakhani force-pushed the bugfix/zl/fix-for-entropy-data-with-spaces branch from 5c719b3 to 9720f05 Compare March 11, 2015 13:24
run(load_fruit_plus_spaces, KeyValGen, _, S=#state{iurls=URLs}) ->
Base = get_base(URLs),
{Key, Val} = KeyValGen(),
Key2 = mochiweb_util:quote_plus(lists:concat([?SPACER, Key])),

Choose a reason for hiding this comment

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

Does this even work? ?SPACER is an atom.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does... but should I just make it a string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

 list_to_binary(lists:concat([' foo bar', binary_to_integer(<<"44">>)])).

would then return <<" foo bar44">>... which would get formatted correctly on the url calls.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

string works too... so whichever is preferred :D.

Choose a reason for hiding this comment

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

I think a string would be less surprising, that's all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@zeeshanlakhani zeeshanlakhani force-pushed the bugfix/zl/fix-for-entropy-data-with-spaces branch from 9720f05 to ca768f3 Compare March 11, 2015 23:40
@seancribbs
Copy link

👍 ca768f3

Test Results:
aae_test-bitcask: pass
---------------------------------------------
0 Tests Failed
1 Tests Passed
That's 100.0% for those keeping score

borshop added a commit that referenced this pull request Mar 13, 2015
…h-spaces

fix to java w/ version update and update to aae/pb tests for RIAK-1557 i...

Reviewed-by: seancribbs
@zeeshanlakhani
Copy link
Contributor Author

@borshop merge

@borshop borshop merged commit ca768f3 into develop Mar 13, 2015
@zeeshanlakhani zeeshanlakhani deleted the bugfix/zl/fix-for-entropy-data-with-spaces branch March 13, 2015 21:25
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.

4 participants