-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
feat(aws-stepfunctions-tasks): allow specifying waitForTaskToken suffix in resourceArn #2686
Changes from 1 commit
e50913e
ccf38b9
9dd544b
5b49cde
38faecd
6dbc5ce
3769f13
d453eed
8788fc3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,26 +2,53 @@ import iam = require('@aws-cdk/aws-iam'); | |
import lambda = require('@aws-cdk/aws-lambda'); | ||
import sfn = require('@aws-cdk/aws-stepfunctions'); | ||
|
||
/** | ||
* Properties for InvokeFunction | ||
*/ | ||
export interface InvokeFunctionProps { | ||
/** | ||
* The JSON that you want to provide to your Lambda function as input. | ||
*/ | ||
readonly payload?: { [key: string]: string }; | ||
|
||
/** | ||
* Whether to pause the workflow until a task token is returned | ||
* | ||
* @default false | ||
*/ | ||
readonly waitForTaskToken?: boolean; | ||
} | ||
|
||
/** | ||
* A StepFunctions Task to invoke a Lambda function. | ||
* | ||
* A Function can be used directly as a Resource, but this class mirrors | ||
* integration with other AWS services via a specific class instance. | ||
*/ | ||
export class InvokeFunction implements sfn.IStepFunctionsTask { | ||
constructor(private readonly lambdaFunction: lambda.IFunction) { | ||
|
||
private readonly waitForTaskToken: boolean; | ||
|
||
constructor(private readonly lambdaFunction: lambda.IFunction, private readonly props: InvokeFunctionProps) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
this.waitForTaskToken = props.waitForTaskToken === true; | ||
} | ||
|
||
public bind(_task: sfn.Task): sfn.StepFunctionsTaskProperties { | ||
return { | ||
resourceArn: this.lambdaFunction.functionArn, | ||
resourceArn: this.waitForTaskToken | ||
albegali marked this conversation as resolved.
Show resolved
Hide resolved
|
||
? 'arn:aws:states:::lambda:invoke.waitForTaskToken' | ||
: this.lambdaFunction.functionArn, | ||
policyStatements: [new iam.PolicyStatement() | ||
.addResource(this.lambdaFunction.functionArn) | ||
.addActions("lambda:InvokeFunction") | ||
], | ||
metricPrefixSingular: 'LambdaFunction', | ||
metricPrefixPlural: 'LambdaFunctions', | ||
metricDimensions: { LambdaFunctionArn: this.lambdaFunction.functionArn }, | ||
parameters: { | ||
FunctionName: this.lambdaFunction.functionName, | ||
...this.props.payload && { Payload: this.props.payload }, | ||
eladb marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would also like an additional validation that if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So, if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually I would like you to make a new function on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I will do the start of this. There is some work I want done in the SFN module anyway. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What you're going to need is in #2706 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That other PR has been merged. You may continue :). Please also have a look at what @wqzoww has written. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hello @rix0rrr only one thing: may I ask you how to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would expect you to call it on the payload structure in the constructor, and throw an error if the function returns There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, i've done it in the latest commit. But tests (unit and integ) are failing now, could you take a look please? |
||
} | ||
}; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,19 +14,33 @@ test('Lambda function can be used in a Task', () => { | |
handler: 'index.hello', | ||
runtime: lambda.Runtime.Python27, | ||
}); | ||
const task = new sfn.Task(stack, 'Task', { task: new tasks.InvokeFunction(fn) }); | ||
const task = new sfn.Task(stack, 'Task', { | ||
task: new tasks.InvokeFunction(fn, { waitForTaskToken: false }) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We usually try not to change existing tests, but rather add new tests. That's a good way to ensure that we didn't break anything. Since |
||
}); | ||
new sfn.StateMachine(stack, 'SM', { | ||
definition: task | ||
}); | ||
|
||
// THEN | ||
expect(stack).toHaveResource('AWS::StepFunctions::StateMachine', { | ||
DefinitionString: { | ||
"Fn::Join": ["", [ | ||
"{\"StartAt\":\"Task\",\"States\":{\"Task\":{\"End\":true,\"Type\":\"Task\",\"Resource\":\"", | ||
{ "Fn::GetAtt": ["Fn9270CBC0", "Arn"] }, | ||
"\"}}}" | ||
]] | ||
"Fn::Join": [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. revert |
||
"", | ||
[ | ||
"{\"StartAt\":\"Task\",\"States\":{\"Task\":{\"End\":true,\"Parameters\":{\"FunctionName\":\"", | ||
{ | ||
Ref: "Fn9270CBC0" | ||
}, | ||
"\"},\"Type\":\"Task\",\"Resource\":\"", | ||
{ | ||
"Fn::GetAtt": [ | ||
"Fn9270CBC0", | ||
"Arn" | ||
] | ||
}, | ||
"\"}}}" | ||
] | ||
] | ||
}, | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this should be "string => any"