Skip to content

Add serverless lambda proxy event type#239

Merged
kolanos merged 4 commits intoiopipe:masterfrom
kolanos:issue/236
May 29, 2018
Merged

Add serverless lambda proxy event type#239
kolanos merged 4 commits intoiopipe:masterfrom
kolanos:issue/236

Conversation

@kolanos
Copy link
Copy Markdown
Contributor

@kolanos kolanos commented May 24, 2018

Closes #236

Signed-off-by: Michael Lavers kolanos@gmail.com

Closes iopipe#236

Signed-off-by: Michael Lavers <kolanos@gmail.com>
@kolanos kolanos requested a review from pselle May 24, 2018 22:22
event_info = {}
for key in self.keys:
value = get_value(self.event, key)
if isinstance(key, tuple):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What's changing that we need to check that these are tuples?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We map one key to another, so keys can now be a list of strings or a list of tuples. If it's a list of tuples, it's a map from old_key to new_key.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why can keys now be of two different types? (List of strings or list of tuples)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Because we map serveless lambda proxy events into api gateway events. See the equivalent JS implementation here:
iopipe/iopipe-js-event-info@2033c3e

Signed-off-by: Michael Lavers <kolanos@gmail.com>
@kolanos kolanos requested a review from pselle May 25, 2018 15:12
event_info = event.collect()
assert event_info != {}

expected_keys = ['@iopipe/event-info.eventType'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Coverage for eventType.source? iopipe/iopipe-js-event-info@2033c3e#diff-ee6f6f22184012610d36277ea8371b4bR87 That is, can we add a test asserting that key/val, for consistency?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't understand the question. The test asserts that the event type produces the expected result (new_key), which is the api gateway keys. This is effectively testing the mapping.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added an assertion to do what I think you're describing.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, I'll hold on reviewing this until after the weekend, as I'm having a hardtime seeing how, programatically, this will result in the slsIntegrationLambda value under eventType.source

Signed-off-by: Michael Lavers <kolanos@gmail.com>
@kolanos kolanos requested a review from pselle May 25, 2018 16:19
Signed-off-by: Michael Lavers <kolanos@gmail.com>
@kolanos kolanos merged commit 472006d into iopipe:master May 29, 2018
@kolanos kolanos deleted the issue/236 branch May 29, 2018 16:06
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.

2 participants