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

Ready: Fix BinaryValue json encoding for MapReduce inputs #655

Merged
merged 4 commits into from
Aug 9, 2016

Conversation

alexmoore
Copy link
Contributor

@alexmoore alexmoore commented Aug 8, 2016

Fixes #520 (CLIENTS-413).

@srgg
Copy link
Contributor

srgg commented Aug 9, 2016

@alexmoore why don't us get rid of BinaryValue as we did for TS?

/**
* Serializes a {@link BinaryValue} to a UTF-8 string for the Riak MapReduce JSON format.
*/
public class BinaryValueSerializer extends JsonSerializer<BinaryValue>
Copy link
Contributor

@srgg srgg Aug 9, 2016

Choose a reason for hiding this comment

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

Should it be registered somehow to be used by default without a necessity to be registered explicitly by user?

UPD:
Found that it is registered in MapReduce.writeSpec, but what about pulling all JsonGenerator creation related stuff into separate static function that will be reused in the Tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done with b101124.

@alexmoore alexmoore changed the title Fix BinaryValue json encoding for MapReduce inputs Ready: Fix BinaryValue json encoding for MapReduce inputs Aug 9, 2016
@christophermancini
Copy link
Contributor

👍 Tests pass, the code looks reasonable but I am having a hard time following the changes. :(

@srgg
Copy link
Contributor

srgg commented Aug 9, 2016

👍

@alexmoore alexmoore merged commit f0082cc into develop Aug 9, 2016
@alexmoore alexmoore deleted the 413-indexmapreduce-json-error branch August 9, 2016 14:35
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.

3 participants