[WIP] Add Data Fixtures for Lambda Event Types #31

naftulikay · 2018-03-27T23:50:47Z

Tests and documentation forthcoming. I am using these data structures for my serverless event bus in Rust using Crowbar.

My lambda handler looks something like this:

lambda!(|event, _context| {
    // dispatch event based on its type
    match serde_json::from_value::<Event>(event) {
        Ok(Event::Auth(event)) => Ok(to_value(&service::auth::route(&context, &event)).unwrap()),
        Ok(Event::CloudWatch(event)) => Ok(service::cloudwatch::route(&context, &event)),
        Ok(Event::Http(event)) => Ok(to_value(&service::http::route(&context, &event)).unwrap()),
        Ok(Event::Records(records)) => Ok(service::multi::route_all(&context, records.entries)),
        Ok(Event::Unknown(event)) => Ok(service::unknown::route(&context, &event)),
        Err(_) => {
            error!("Unable to convert event.");
            Ok(json!({ "statusCode": 400 ,"message": "Unable to convert event." }))
        }
    }
});

Thus, I do routing based on event type. For example, SNS:

pub fn route(context: &Context, record: &sns::Record) {
    info!("Received SNS Event: {}", record.event.message_id);

    match from_str(&record.event.message) {
        Ok(SnsEventKind::Ses(message)) => service::ses::receive(&context, &message),
        Ok(SnsEventKind::Unknown(value)) => warn!("Unknown JSON Event: {}", to_string_pretty(&value).unwrap()),
        Err(_) => error!("Unknown non-JSON event: {}", record.event.message),
    };
}

My multi-event router:

/// Dispatch multiple events.
pub fn route_all(context: &Context, events: Vec<Record>) -> Value {
    for event in events {
        match event {
            Record::S3(event_record) => s3::route(&context, &event_record),
            Record::Ses(event_record) => {
                // if it's an SES request/response, a response may be required, so only process one
                // of them if the type is RequestResponse
                match event_record.event.receipt.action {
                    Action::Lambda(ref action) if action.invocation_type == RequestResponse => {
                        return to_value(ses::filter(&context, &event_record)).unwrap();
                    },
                    _ => ses::route(&context, &event_record.event),
                };
            },
            Record::Sns(event_record) => sns::route(&context, &event_record),
            Record::Unknown(value) => {
                error!("Unknown event record type: {}", to_string_pretty(&value).unwrap())
            }
        };
    }

    Value::Null
}

This dispatching occurs in around 400-500 microseconds and works extremely well to make things type safe.

Obviously there's a lot here and it's not done. If @ilianaw or others have any input, RFC is open to comments 😃

Tests and documentation forthcoming. I am using these data structures for my serverless event bus in Rust using Crowbar.

iliana · 2018-03-28T00:02:39Z

I like this. :)

Can you add what has been done and what remains as checkboxes to the pull request description so we can see the path to this leaving a WIP state?

naftulikay · 2018-03-28T18:54:13Z

🎉 so happy that you're welcoming this PR. I will create a separate issue and track things there. There are so many different data types to support, so this PR will maybe only cover a subset with future on the way, hope that's okay.

iliana · 2018-03-28T20:44:01Z

Yeah, I don't have an issue with incremental development over time.

I'm hoping I'll have a chance to do a good read through of all of this soon.

naftulikay · 2018-03-28T21:16:06Z

@ilianaw can you please go turn on Travis CI for this project? I have updated with a build badge and Travis configuration for building and testing.

EDIT: Actually, I will submit Travis support in a separate PR.

naftulikay · 2018-03-29T00:35:36Z

Latest adds support for CloudWatch AutoScaling events.

softprops · 2018-03-31T03:39:56Z

I'd love to see this branch ship fast with additional followups to add other events types. Commented with extra thoughts on the checkbox ticket

softprops · 2018-03-31T04:00:06Z

src/data/apigateway/mod.rs

+#[derive(Serialize,Deserialize)]
+pub struct HttpEvent {
+    pub body: Option<String>,
+    pub headers: Option<HashMap<String, String>>,


One small thought. I did something similar initially in a separate project and found that the extra option type to be a bit annoying from a usability perspective. Espectially those new to rust

9 | println!("{:?}", ev.query_string_parameters.and_then(|m| m.get("foo"))); | ---------------------------------------------------------^-------------- | | | | | | | `m` dropped here while still borrowed | | borrowed value does not live long enough | borrowed value needs to live until here

Was't a very great experience.

I found a way to deserialize the nullable maps into default values greatly improved usability for new comers

as well as reduced verbosity while retaining the same level of rust runtime saftey.

println!("{:?}", ev.query_string_parameters.and_then(|m| m.get("foo")))

println!("{:?}", ev.query_string_parameters.get("foo"));

Yes, let's deserialize things like maps and vectors into default (empty) values.

I really think that kind of thing helps the ergonomics for people using this.

softprops · 2018-03-31T04:01:58Z

src/data/apigateway/mod.rs

+}
+
+#[derive(Serialize,Deserialize)]
+pub struct HttpEventRequestContext {


I think you can largely get a way with a struct level #[serde(rename_all = "camelCase")] here

note: comments like this are not really blockers for me and could easily fit into followups

softprops · 2018-03-31T04:04:31Z

src/data/apigateway/mod.rs

+
+impl HttpResponse {
+
+    pub fn empty(status: HttpStatus) -> Self {


I'm trying to convince myself to drop an existing external effort to add types to crowbar. Something I'd be missing here is a Default impl

softprops · 2018-03-31T04:12:17Z

src/data/apigateway/mod.rs

+    }
+}
+
+serde_aux_enum_number_declare!(HttpStatus {


I'd maybe suggest not going too overboard with type mappings here. I see crowbar being something as a low level mapping to raw primitive types, in this case a u16 would do. The reasoning behind this line of thinking is that you could then more easily adapt to and not duplicate efforts to create a typed interface like http on top of crowbar. As is you couldn't build a mapping to the http crate on top of this because if the incomplete mapping of http statuses. I'd much prefer the crowbar retain the most primitive types and enable layering of extra layers on top in another crate.

So should we rely on another crate for this? I do actually like having strongly typed things.

I'd actually not recommend adding any more crate dependencies on crowbar. I don't see crowbar as a framework but as an faithful representation of lambda. I'm thinking now it its really api gateway, the service, that defines what status codes you can dispatch lambdas on so that may actually be a more restrictive set. I redact my previous comment!

I agree that a u16 would do for the HTTP status.

I experimented a lot with the http crate for another project involving Lambda@Edge. It helped with initial development but I eventually ripped it out because it was doing a lot of validation that CloudFront already does for you.

softprops · 2018-03-31T04:14:13Z

src/data/apigateway/mod.rs

+impl HttpEventRequestContext {
+
+    pub fn time(&self) -> Option<DateTime<Utc>> {
+        // Utc::datetime_from_str(&self.request_time, "").ok()e


you could probably ditch a hard coupling of crowbar to a crate by letting it be a user decision to select a time parsing crate and let crowbar be smaller.

How can we genericize this to work like that?

If we didn't want to represent this as a string that lambda events do, you could maybe use a feature + cfg. I don't feel really strongly about this. Only that I'd like to see crowbar have a solve only the minimal set of problems with a minimal set of dependencies. Motivations are that it lets users pick a crate they prefer for representing time typically so that they only have one crate for dealing with time in their projects ( think about what hyper did with tls ) and it frees up crowbar up from a bit of maintenance by not worrying falling behind on its dependencies. I also don't feel too strongly here but that's my two cents. I'd feel otherwise if rust had a std lib way of representing timestamps.

I don't think genericizing the time crate is necessary to land this PR.

softprops · 2018-03-31T04:40:46Z

src/data/apigateway/mod.rs

+
+impl HttpEvent {
+
+    pub fn get_header(&self, key: &str) -> Option<&str> {


There are some some growing community recommendations on avoiding get_ where you can https://rust-lang-nursery.github.io/api-guidelines/naming.html#getter-names-follow-rust-convention-c-getter

Instead a simple .header("Foo") will do

Roger that.

softprops · 2018-03-31T05:47:54Z

src/data/ses/mod.rs

+
+impl Response {
+
+    pub fn from(action: ResponseAction) -> Self {


Why not impl the From trait instead?

Let's do it 👍

naftulikay · 2018-04-02T03:44:01Z

I will start breaking this apart into a PR per service supported, I think that's probably the most prudent course of action.

One thing that is probably a really good idea is to find a way to efficiently nest CloudWatch style events and deserialize them more efficiently than I'm currently doing.

softprops · 2018-04-02T04:01:03Z

One thing that is probably a really good idea is to find a way to efficiently nest CloudWatch style events and deserialize them more efficiently than I'm currently doing.

Can you link to an example in this pull or sketch out in a comment what you mean?

iliana · 2018-04-06T16:58:00Z

Overall I'm happy with this, and thanks to @softprops for the additional review.

I'm fine with continuing with this PR, or breaking it apart as a PR per service (the latter would be great as its easier for others to help write the structs out).

I have two thoughts:

What happens when AWS inevitably adds another field to these objects? Does serde correctly ignore extra fields?
Would it be reasonable to make all of these an enabled-by-default feature to allow folks who aren't using this to disable it and speed up compile times?

softprops · 2018-04-07T17:41:14Z

What happens when AWS inevitably adds another field to these objects? Does serde correctly ignore extra fields?

Yes. This used to not be true but is now a default of serde.

naftulikay · 2018-04-07T18:39:58Z

I apologize for my lateness, just getting back from vacation. I will get caught up this week.

iliana · 2018-04-07T20:14:58Z

No worries! ^_^;

naftulikay · 2018-04-08T20:46:13Z

TL;DR the events received from Lambda can come via a few sources:

SNS: a fairly consistent data type; you'd basically just deserialize the message property into the struct(s) that you expect.
CloudWatch: there are a lot (Create Data Mappings for Lambda Input and Output Events #32) of different CloudWatch data types.
API Gateway: requests, auth requests, and responses.

Therefore, our enums for representing all kinds of incoming events should basically include these three, and then users can disambiguate between the implementations (ie cloudwatch::BatchEvent, cloudwatch::ApiCallEvent, apigateway::Request, apigateway::AuthRequest, etc.)

I will probably break this PR down into tiny pieces and make one PR for each service to add. I wholeheartedly agree with removing Option<T> wherever possible and using the default for things like Vec<T> and BTreeMap<K, V>.

softprops · 2018-09-12T05:36:04Z

Is there any way to help move this forward? I'd love to help

[WIP] Add Data Fixtures for Lambda Event Types

39b1ea6

Tests and documentation forthcoming. I am using these data structures for my serverless event bus in Rust using Crowbar.

naftulikay force-pushed the feature/data branch from 5c57ad7 to 39b1ea6 Compare March 27, 2018 23:53

naftulikay mentioned this pull request Mar 28, 2018

Create Data Mappings for Lambda Input and Output Events #32

Open

47 tasks

naftulikay added 2 commits March 28, 2018 14:10

refactor to different services

c202726

Travis CI and Badge

273967b

naftulikay added 4 commits March 28, 2018 15:43

we have autoscaling lifecycle actions

83983e8

all them autoscaling bajanguses

970a7b7

damn

f593074

test disambiguation

41505cb

naftulikay added 3 commits March 28, 2018 17:55

api call implementation

ea93b5b

tests breaking, working on batch

caec126

passing tests for cloudwatch batch events

f658859

softprops reviewed Mar 31, 2018

View reviewed changes

naftulikay mentioned this pull request Dec 4, 2018

Documentation for Various Lambda Use-Cases awslabs/aws-lambda-rust-runtime#29

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Add Data Fixtures for Lambda Event Types #31

[WIP] Add Data Fixtures for Lambda Event Types #31

naftulikay commented Mar 27, 2018

iliana commented Mar 28, 2018

naftulikay commented Mar 28, 2018

iliana commented Mar 28, 2018

naftulikay commented Mar 28, 2018 •

edited

Loading

naftulikay commented Mar 29, 2018

softprops commented Mar 31, 2018

softprops Mar 31, 2018

naftulikay Apr 2, 2018

softprops Apr 2, 2018

softprops Mar 31, 2018

softprops Apr 2, 2018

softprops Mar 31, 2018

naftulikay Apr 2, 2018

softprops Mar 31, 2018

naftulikay Apr 2, 2018

softprops Apr 2, 2018

iliana Apr 6, 2018

softprops Mar 31, 2018

naftulikay Apr 2, 2018

softprops Apr 2, 2018

iliana Apr 6, 2018

softprops Mar 31, 2018

naftulikay Apr 2, 2018

softprops Mar 31, 2018

naftulikay Apr 2, 2018

naftulikay commented Apr 2, 2018

softprops commented Apr 2, 2018

iliana commented Apr 6, 2018

softprops commented Apr 7, 2018

naftulikay commented Apr 7, 2018

iliana commented Apr 7, 2018

naftulikay commented Apr 8, 2018

softprops commented Sep 12, 2018


		impl HttpResponse {

		pub fn empty(status: HttpStatus) -> Self {


		impl HttpEvent {

		pub fn get_header(&self, key: &str) -> Option<&str> {


		impl Response {

		pub fn from(action: ResponseAction) -> Self {

[WIP] Add Data Fixtures for Lambda Event Types #31

Are you sure you want to change the base?

[WIP] Add Data Fixtures for Lambda Event Types #31

Conversation

naftulikay commented Mar 27, 2018

iliana commented Mar 28, 2018

naftulikay commented Mar 28, 2018

iliana commented Mar 28, 2018

naftulikay commented Mar 28, 2018 • edited Loading

naftulikay commented Mar 29, 2018

softprops commented Mar 31, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naftulikay commented Apr 2, 2018

softprops commented Apr 2, 2018

iliana commented Apr 6, 2018

softprops commented Apr 7, 2018

naftulikay commented Apr 7, 2018

iliana commented Apr 7, 2018

naftulikay commented Apr 8, 2018

softprops commented Sep 12, 2018

naftulikay commented Mar 28, 2018 •

edited

Loading