feat: serde dynamo #40
Conversation
| let products: Vec<Product> = match res.items { | ||
| Some(items) => from_items(items).map_err(|_| | ||
| // TODO: Find out correct error from underlying error? | ||
| Error::InternalError("Missing name"))?, |
There was a problem hiding this comment.
This already shows a problem: How to map the underlying error to our very specific error Missing name. I don't think that the underlying serde errors give us that info: https://github.com/zenlist/serde_dynamo/blob/3.0.0-alpha/src/error.rs
There was a problem hiding this comment.
Looking into the source code, it looks like it just uses serde_dynamo::ErrorImpl::Message, which might contain the right message - but that's not a guarantee. The best thing would be to write a small sample using the Product struct and serde_dynamo.
| @@ -69,7 +68,8 @@ impl StoreGet for DynamoDBStore { | |||
| .await?; | |||
|
|
|||
| Ok(match res.item { | |||
| Some(item) => Some(item.try_into()?), | |||
| // TODO: Find out correct error from underlying error? | |||
| Some(item) => from_item(item).map_err(|_| Error::InternalError("Missing name"))?, | |||
There was a problem hiding this comment.
Same problem as above.
| @@ -84,7 +84,8 @@ impl StorePut for DynamoDBStore { | |||
| self.client | |||
| .put_item() | |||
| .table_name(&self.table_name) | |||
| .set_item(Some(product.into())) | |||
| // TODO: Can this fail? | |||
| .set_item(Some(to_item(product).unwrap())) | |||
There was a problem hiding this comment.
We still have a unit test for that, so we tested that to_item can serialize a product.
There was a problem hiding this comment.
That shouldn't fail since all the fields in a product map to valid DynamoDB attributes.
| let next = res.last_evaluated_key.map(|m| m.get_s("id").unwrap()); | ||
| let next = res | ||
| .last_evaluated_key | ||
| .map(|m| from_attribute_value(m["id"].clone()).unwrap()); |
There was a problem hiding this comment.
Not sure if there is a more elegant way to write this?
|
Hey @hffmnn! Thanks a lot for creating this! I think given that |
🔥 Don't merge: this is rather experimental and up for discussion!
🔥 Note: So far I didn't deploy the API with theses changes so I am not sure, if it works as expected.
No issue, I wanted to try out
serde_dynamocrate with the experimental support for the official aws-sdk.Description of changes:
serde_dynamofrom_itemandfrom_itemsand remove handcraftedFromandTryFromimplementationsserde_dynamofrom_attribute_valueand removesrc/store/dynamodb/ext.rs