search

aerospike / aerospike-client-rust

Rust client for the Aerospike database
https://www.aerospike.com/
Other
82 stars 29 forks source link

Serde Serializers for Record and Value objects #85

Closed jonas32 closed 4 years ago

jonas32 commented 4 years ago

This PR adds the widely used Serializer for Serde to the Value, Record, Bins, Key and BatchRead structs. (Serializer in value.rs made by @j-brn) Its useful when using the Aerospike Client for REST APIs returning JSON with for example actix and does not affect the performance in any way if not used. Without, you have to build own structs around the Value object and fill them with big match blocks. Many Database drivers include it for that reason (for example Postgres and MongoDB).

jonas32 commented 4 years ago

Ok, i feature gated the whole serialization content into the "serialization" feature. You will need cargo build --features "serialization" or cargo test --features "serialization" to include it. Probably we should adjust CI to build once without and once with the feature. I didnt do it yet because i saw some messenger hooks on it and didnt want to spam the whole team when testing.

jhecking commented 4 years ago

Ok, i feature gated the whole serialization content into the "serialization" feature. You will need cargo build --features "serialization" or cargo test --features "serialization" to include it.

Awesome! Thanks.

Probably we should adjust CI to build once without and once with the feature. I didnt do it yet because i saw some messenger hooks on it and didnt want to spam the whole team when testing.

Agree. And you can go ahead and update .travis.yaml to add the additional testing steps.

jonas32 commented 4 years ago

Would you like to run the feature as an independent build job or just run the test --features command right after the normal test command in the same script section?

jhecking commented 4 years ago

Good question. I think it will probably be faster to run both tests in the same Travis job, so let's do that.

jonas32 commented 4 years ago

I updated both AppVeyor and Travis. AppVeyor only runs the cargo test for both and Travis runs build, test and README.md test for both. Running all 3 steps twice in Travis might be redundant, but i think its the safer way (see the failed Travis build before at 59987c9)

jhecking commented 4 years ago

I'm going to go ahead and merge this, unless you were planning on making any further changes?

jonas32 commented 4 years ago

You can go on. This features is finished.