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

http-dynamodb example #562

Merged
merged 10 commits into from Nov 8, 2022
Merged

http-dynamodb example #562

merged 10 commits into from Nov 8, 2022

Conversation

jknoetzke
Copy link
Contributor

Issue #, if available:
This is an example of a rust lambda function that takes in a JSON payload and writes to a Dynamodb table.

Description of changes:
First commit.

By submitting this pull request

  • [x ] I confirm that my contribution is made under the terms of the Apache 2.0 license.
  • [x ] I confirm that I've made a best effort attempt to update all relevant documentation.

Copy link
Contributor

@calavera calavera left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this contribution! I left you some comments to cleanup the code a little bit.

examples/http-dynamodb/Cargo.toml Outdated Show resolved Hide resolved
examples/http-dynamodb/src/main.rs Outdated Show resolved Hide resolved
examples/http-dynamodb/src/main.rs Show resolved Hide resolved
examples/http-dynamodb/src/main.rs Outdated Show resolved Hide resolved
examples/http-dynamodb/src/main.rs Outdated Show resolved Hide resolved
examples/http-dynamodb/src/main.rs Outdated Show resolved Hide resolved
examples/http-dynamodb/src/main.rs Outdated Show resolved Hide resolved
examples/http-dynamodb/src/main.rs Outdated Show resolved Hide resolved
examples/http-dynamodb/src/main.rs Outdated Show resolved Hide resolved
@jknoetzke jknoetzke closed this Nov 6, 2022
@jknoetzke jknoetzke reopened this Nov 6, 2022
@jknoetzke
Copy link
Contributor Author

Hello

Thanks for all the useful comment.

I have attempted to implement all of the requested changes.
There is one requested change I had difficulty with and this is regards to returning the struct as JSON to the user after it was pushed into the table.

In the end, I created two Item structs using serde, one that gets sent to Dynamo and one that is sent back to the user. I doubt this is the correct way. However, add_item requires ownership of the struct and with serde (serialize , deserialize) I could not figure out a way to implement the Copy or Move trait.. The compiler was not allowing me to reuse Item to return to the user as it had been changed.. So, I implemented this hack instead.

I would also like to return a 400 error if the rest call made by the user is missing parameters.. Now the function crashes and returns a 500. Any ideas how to do this ? I see examples that you wrote in how to return errors.. But I can't figure out how to determine when serde realizes the request is missing a field and to return the missing field(s) back to the user..

Thanks.. getting there.

Copy link
Contributor

@calavera calavera left a comment

Choose a reason for hiding this comment

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

this is looking really nice! I just left a minor comment to be consistent with formatting.

pub async fn add_item(client: &Client, item: Item, table: &String) -> Result<(), OtherError> {
let user_av = AttributeValue::S(item.username);
pub async fn add_item(client: &Client, item: Item, table: &str) -> Result<(), OtherError>
{ let user_av = AttributeValue::S(item.username);
Copy link
Contributor

Choose a reason for hiding this comment

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

can you run cargo fmt?

//Serialize the data into the struct.
let item: Item = serde_json::from_str(s).map_err(Box::new)?;
//Create a copy to send back via the Response.
let copy_item: Item = serde_json::from_str(s).map_err(Box::new)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

since you're basically cloning the struct, it might be better to add Clone to the structure's derive attributes, and use item.clone() when you call add_item.

@jknoetzke
Copy link
Contributor Author

Code was formatted.
Implemented clone on the struct.
Removed duplicate variable for the struct.

use aws_sdk_dynamodb::model::AttributeValue;
use aws_sdk_dynamodb::{Client, Error as OtherError};
use lambda_http::{run, service_fn, Error, IntoResponse, Request};
use tracing::info;

#[derive(serde::Deserialize, serde::Serialize)]
Copy link
Contributor

Choose a reason for hiding this comment

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

add

Suggested change
#[derive(serde::Deserialize, serde::Serialize)]
#[derive(Clone, serde::Deserialize, serde::Serialize)]

Comment on lines 15 to 25
impl Clone for Item {
fn clone(&self) -> Item {
return Item {
p_type: self.p_type.clone(),
age: self.age.clone(),
username: self.username.clone(),
first: self.first.clone(),
last: self.last.clone(),
};
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you need this if you add the derive to the struct.

Suggested change
impl Clone for Item {
fn clone(&self) -> Item {
return Item {
p_type: self.p_type.clone(),
age: self.age.clone(),
username: self.username.clone(),
first: self.first.clone(),
last: self.last.clone(),
};
}
}

@jknoetzke
Copy link
Contributor Author

Thanks. I had tried adding Clone to derive, and the compiler wasn't having it.. I thought maybe serde was somehow blocking Clone.. but it looks like I just had the syntax wrong. ;-)

I've made the change.

There is one last thing, should I be checking if the input request is correctly formatted and missing fields ? Right now, if you send a request with a missing field, the function crashes and returns a 500.

I can see how to return an error from your examples, but I can't figure out how to pull out the errors from serde when calling let item: Item = serde_json::from_str(s).map_err(Box::new)?;

@jknoetzke
Copy link
Contributor Author

Check if JSON is valid. If not, return 400 and abort.
Returns 200 if record is inserted into table.

Copy link
Contributor

@calavera calavera left a comment

Choose a reason for hiding this comment

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

Thanks for adding this example, I think it's good to be merged now! 🎊

@calavera calavera merged commit 996fb9b into awslabs:main Nov 8, 2022
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