Skip to content

Conversation

@jaymccon
Copy link
Contributor

@jaymccon jaymccon commented Nov 27, 2019

Issue #, if available: Also fixed #60

Description of changes:

completes support for async handlers. I've end-to-end tested async CREATE using both local and cloudwatch reinvocations.

I optimised for getting this pr in as quickly as possible, over making the code pristine. This gets the plugin fully functional and can iterate/improve from here as we go.

Also fixes a few small bug that were exposed during testing:

  • properly serialize ProgressEvent responses (was getting Internal Failure errors)
  • log_delivery was creating duplicate handlers (duplicate messages delivered) when lambda re-used a container
  • cloudwatch rules/targets were not being cleaned up

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@jaymccon jaymccon marked this pull request as ready for review November 27, 2019 23:32
Copy link
Contributor

@tobywf tobywf left a comment

Choose a reason for hiding this comment

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

looks good

if "callbackContext" in ser:
del ser["callbackContext"]
if self.errorCode:
ser["errorCode"] = self.errorCode.name
Copy link
Contributor

Choose a reason for hiding this comment

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

fine for now, i think in future we'll want to have another class to be the cloudformation callback struct and have a method to convert from one to the other

@tobywf
Copy link
Contributor

tobywf commented Nov 28, 2019

Also fixes #60

parsed = self._parse_request(event_data)
caller_sess, platform_sess, request, action, callback, event = parsed
# Acknowledge the task for first time invocation
if not event.requestContext:
Copy link
Contributor

Choose a reason for hiding this comment

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

@tobywf tobywf merged commit d1d209f into aws-cloudformation:master Nov 29, 2019
tobywf added a commit that referenced this pull request Dec 3, 2019
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.

Log something when catching exceptions outside of the handler

3 participants