Fixed / Added Option support for: String, i32, i64, f32, f64, uuid, date, and bool #129
Conversation
| @@ -339,6 +339,7 @@ impl std::convert::TryFrom<GValue> for uuid::Uuid { | |||
| fn try_from(value: GValue) -> GremlinResult<Self> { | |||
| match value { | |||
| GValue::Uuid(uid) => Ok(uid), | |||
| GValue::List(s) => from_list(s), | |||
There was a problem hiding this comment.
Having to add these GValue::List matches kind of surprised me. Seems like if we're being sent back a List element something should be wrong? But from stepping through with a debugger we end up here from the impl_try_from_option macro which seems to mean it's the GValue coming from Tinkerpop? 🤔
There was a problem hiding this comment.
Stepping through with the debugger a little earlier, basically before it creates the properties Map I then deserialize values out of it seems to flow from deserialize_map to deserialize_list to deserialize_XXX where XXX are the various deserialize methods in https://github.com/wolf4ood/gremlin-rs/blob/master/gremlin-client/src/io/serializer_v3.rs does that seem correct / working as intended @wolf4ood ?
There was a problem hiding this comment.
Yeah it' seems strange that you had to add the list variant let me check :)
There was a problem hiding this comment.
Probably it has something to do with multivalue properties but let me recall in my head. The try from stuff was added for doing automatic structs binding :)
There was a problem hiding this comment.
It seemed like it was creating singleton lists in the debugger when I was walking through the steps and then unpacking the single item out of the list. I'm working on #127 now so I may have more insight as make my way through it if it is a result of the multi-value properties.
There was a problem hiding this comment.
If I remember correctly when doing value map step the map contains the values in form of array for supporting multi-properties on vertex type
| } | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| impl_try_from_option!(String); | ||
| impl_try_from_option!(i32); |
There was a problem hiding this comment.
Ideally we'd be able to invoke this macro with some kind of loop over invoking the macro? I'm going to be repeating all these value types again for #127 I believe, so if there's a way to do a loop let me know.
| @@ -375,6 +378,36 @@ impl std::convert::TryFrom<GValue> for bool { | |||
| } | |||
| } | |||
|
|
|||
| impl std::convert::TryFrom<GValue> for f32 { | |||
There was a problem hiding this comment.
Just a heads up this is a duplicate implementation of the same code in https://github.com/wolf4ood/gremlin-rs/pull/131/files to keep the PRs independent for review and merging. One of the implementations will need to be removed.
| } | ||
| } | ||
|
|
||
| impl std::convert::TryFrom<GValue> for f64 { |
There was a problem hiding this comment.
Just a heads up this is a duplicate implementation of the same code in https://github.com/wolf4ood/gremlin-rs/pull/131/files to keep the PRs independent for review and merging. One of the implementations will need to be removed.
|
Thanks @criminosis :) |
Closes #126