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

de/serialize functions #86

Merged
merged 7 commits into from
Jun 5, 2015
Merged

de/serialize functions #86

merged 7 commits into from
Jun 5, 2015

Conversation

rclark
Copy link
Contributor

@rclark rclark commented Jun 5, 2015

  • Adds serialize function (takes a dyno item, converts it to wire format, JSON.stringifies it)
  • Adds deserialize function (takes a string from above, JSON.parses it, converts it to dyno format)

cc @willwhite feel free to try and add more "tricky" cases to the test. The difficult part of all this is dealing with buffer objects, and turning them into round-tripable strings

@@ -28,9 +28,37 @@ Dyno.multi = function(readConfig, writeConfig) {
};

var types = require('./lib/types');
Dyno.createSet = types.createSet;
Dyno.toDynamoTypes = types.toDynamoTypes;
Dyno.typesFromDynamo = types.typesFromDynamo;

Choose a reason for hiding this comment

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

@rclark, the only problem with removing typesFromDynamo is that dynamodb-replicator is currently calling dyno.getItem with the wire-formatted Key that it reads from the kinesis stream. Since dyno.getItem no longer supports wire-formatted inputs (ref #85 ), and we don't know the structure of the key beforehand, we'll have to call typesFromDynamo on the Key before calling dyno.getItem with it. I imagine there are plenty of other use cases for it too.

cc/ @willwhite

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Caught this in mapbox/dynamodb-replicator@ce342ab by removing dyno from the lambda function entirely.

I'm on the fence about exposing these conversion functions. I'm with you in that I can imagine other use cases, but I'm leaning towards keeping them internal until some of those use-cases become clearly un-imaginary.

Choose a reason for hiding this comment

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

Yeah, when I wrote this I hadn't seen mapbox/dynamodb-replicator@ce342ab, and I forgot about just using straight-up AWS SDK. We'll wait until other use cases arrive.

rclark pushed a commit that referenced this pull request Jun 5, 2015
@rclark rclark merged commit b90dd8b into types Jun 5, 2015
@rclark rclark deleted the serialization branch June 5, 2015 18:15
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.

None yet

2 participants