Initial lambda wrapper 2 #4
Conversation
rjlohan
force-pushed
the
initial-lambda-wrapper-2
branch
2 times, most recently
from
433368c to
6432429
Compare
src/com/aws/cfn/GenericHandler.java
Outdated
| Action action, | ||
| RequestContext context) { | ||
|
|
||
| Gson gson = new GsonBuilder().create(); |
There was a problem hiding this comment.
just wondering, what was the reasoning for picking Gson over Jackson?
There was a problem hiding this comment.
No specific reason. But it works.
| request.getAction(), | ||
| (endTime.getTime() - startTime.getTime())); | ||
|
|
||
| // When the handler responses InProgress with a callback delay, we trigger a callback to re-invoke |
There was a problem hiding this comment.
if the handler wants to record additional progress events, how would it do that?
src/com/aws/cfn/LambdaWrapper.java
Outdated
| return null; | ||
| } | ||
|
|
||
| public void handleRequest(InputStream inputStream, OutputStream outputStream, Context context) throws IOException { |
There was a problem hiding this comment.
where is the POJO which contains the ResourceRequest we get from CloudFormation?
I don't see clientRequestToken or credentials anywhere.
Also, resourceTypeVersion, nextToken for List calls, previousResourceProperties, behaviorSpec
There was a problem hiding this comment.
This needs iteration and refactoring to consume the actual handler interface and context object. So far, just a Lambda wrapper for a basic function.
|
|
||
| public void publishInvocationMetric(final Date timestamp, final Action action) { | ||
| publishMetric( | ||
| Metrics.METRIC_NAME_HANDLER_INVOCATION_COUNT, |
There was a problem hiding this comment.
how is this meant to work? invocations by the handler or by cloudwatch? if both, how would we keep track?
There was a problem hiding this comment.
This metric is indicating the handler is running. CloudFormation will perform the initial invocation, but since the wrapper re-invokes itself, there may be many more actual invokes are CFN hands off.
| * If the request was the result of a CloudWatchEvents re-invoke trigger the | ||
| * CloudWatchEvents Trigger Id is stored to allow cleanup | ||
| */ | ||
| private String cloudWatchEventsTargetId; |
There was a problem hiding this comment.
why would the handler implementation care about the cloudWatch stuff or the invocation count?
There was a problem hiding this comment.
This RequestContext object comes unto Lambda, it's not passed to the Handlers.
There was a problem hiding this comment.
your example takes in RequestContext and it appears you're passing it here? https://github.com/awslabs/aws-cloudformation-rpdk-java-plugin/blob/a83bb79089e5604144d91aa18bd9a2cf68875a13/src/com/aws/cfn/LambdaWrapper.java#L174
This commit includes a basic framework for a Java Lambda wrapper for RPDK handlers. It includes some temporary files and types which need to be refactored out in favour of the actual RPDK handler implementations. No tests exist yet, given the expected refactoring which will happen.
rjlohan
force-pushed
the
initial-lambda-wrapper-2
branch
from
6432429 to
a83bb79
Compare
|
|
||
| final Gson gson = new GsonBuilder().create(); | ||
| final JsonObject jsonObject = gson.toJsonTree(request.getResourceModel()).getAsJsonObject(); | ||
| final ResourceModel model = gson.fromJson(jsonObject.toString(), ResourceModel.class); |
There was a problem hiding this comment.
was going to ask if we can do this stuff for them; it looks like you half way did it when you declared the argument as HandlerRequest<ResourceModel>
| public void handleRequest(final InputStream inputStream, | ||
| final OutputStream outputStream, | ||
| final Context context) throws IOException { | ||
| this.logger = context.getLogger(); |
There was a problem hiding this comment.
this whole function should be surrounded in try/catch/finally and return some response to CloudFormation to know when it fails (like with an NPE or some other bug of ours), and record metrics (I'd say metrics stuff can be done in a later PR, but I think the try/catch and write response should be in this one)
There was a problem hiding this comment.
I've raised #8 to do this later, once the interface back to the caller is cleaned up a bit.
…brianlao-github-fort Fix resource targetting for a stack level hook
Initial Lambda Wrapper Implementation
This commit includes a basic framework for a Java Lambda wrapper for RPDK handlers.
It includes some temporary files and types which need to be refactored out in favour
of the actual RPDK handler implementations.
No tests exist yet, given the expected refactoring which will happen.
This PR also drops an initial implementation for #2
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.