fix(stepfunctions-tasks): runtime language used to evaluate expressions is ignored #30302

colifran · 2024-05-22T05:42:41Z

Reason for this change

EvaluateExpression exposes a runtime property that can be used to configure the runtime language used to evaluate an expression. When the handler for this was migrated into the handler framework we hid the runtime property and didn't make it configurable. As a result, when the runtime property is specified as part of EvaluateExpressionProps it ends up being dropped in place of the code generated runtime.

Description of changes

Added a configurable runtime property to the generated EvalNodejsSingletonFunctionProps interface and set this property using runtime property on EvaluateExpressionProps if one was provided. Otherwise, the current node 18 default is used.

Description of how you validated changes

Unit test for codegen with eval-nodejs-provider. Integ test for default EvaluateExpression runtime (we already test a configurable runtime, unfortunately this was the same as the default so this bug was not caught).

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

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

Signed-off-by: Francis <colifran@amazon.com>

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

Signed-off-by: Francis <colifran@amazon.com>

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

Signed-off-by: Francis <colifran@amazon.com>

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

aws-cdk-automation · 2024-05-22T06:48:24Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: aecd869
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

bergjaak · 2024-05-22T11:48:53Z

...amework-integ/test/aws-stepfunctions-tasks/test/integ.evaluate-expression-default-runtime.ts

+          new sfn.Wait(this, 'Wait', {
+            time: sfn.WaitTime.secondsPath('$.d'),
+          }),


is there a reason to add a wait? Just wondering since it will make the test slower

bergjaak · 2024-05-22T11:56:28Z

...resource-handlers/test/custom-resources-framework/expected/singleton-function-eval-nodejs.ts

+import { Construct } from "constructs";
+import * as lambda from "../../../aws-lambda";
+
+export class EvalNodejsSingletonFunction extends lambda.SingletonFunction {


Just for my understanding, is this used in packages/@aws-cdk/custom-resource-handlers/test/custom-resources-framework/expected/singleton-function-eval-nodejs.ts? If not, what's the purpose of this?

Yeah. It's just a template to use as "expected" to compare against a result in a unit test.

mergify · 2024-05-22T15:36:45Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

…ns is ignored (aws#30302) ### Reason for this change `EvaluateExpression` exposes a runtime property that can be used to configure the runtime language used to evaluate an expression. When the handler for this was migrated into the handler framework we hid the runtime property and didn't make it configurable. As a result, when the runtime property is specified as part of `EvaluateExpressionProps` it ends up being dropped in place of the code generated runtime. ### Description of changes Added a configurable runtime property to the generated `EvalNodejsSingletonFunctionProps` interface and set this property using runtime property on `EvaluateExpressionProps` if one was provided. Otherwise, the current node 18 default is used. ### Description of how you validated changes Unit test for codegen with eval-nodejs-provider. Integ test for default `EvaluateExpression` runtime (we already test a configurable runtime, unfortunately this was the same as the default so this bug was not caught). ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

aws-cdk-automation · 2024-07-25T21:06:32Z

Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one.

expose evaluate nodejs provider runtime property

5d40ee1

Signed-off-by: Francis <colifran@amazon.com>

github-actions bot added the p2 label May 22, 2024

aws-cdk-automation requested a review from a team May 22, 2024 05:42

mergify bot added the contribution/core This is a PR that came from AWS. label May 22, 2024

aws-cdk-automation previously requested changes May 22, 2024

View reviewed changes

colifran added 3 commits May 21, 2024 22:48

unit test for framework

955c42a

Signed-off-by: Francis <colifran@amazon.com>

fix expected

56887ee

Signed-off-by: Francis <colifran@amazon.com>

docstring

19b5646

Signed-off-by: Francis <colifran@amazon.com>

colifran added the pr-linter/exempt-integ-test The PR linter will not require integ test changes label May 22, 2024

colifran removed the pr-linter/exempt-integ-test The PR linter will not require integ test changes label May 22, 2024

aws-cdk-automation previously requested changes May 22, 2024

View reviewed changes

colifran added 2 commits May 21, 2024 23:11

fix unit test

7953745

Signed-off-by: Francis <colifran@amazon.com>

snaps

aecd869

Signed-off-by: Francis <colifran@amazon.com>

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label May 22, 2024

bergjaak reviewed May 22, 2024

View reviewed changes

bergjaak approved these changes May 22, 2024

View reviewed changes

mergify bot merged commit dfea721 into main May 22, 2024
20 checks passed

mergify bot deleted the colifran/eval-expression-runtime branch May 22, 2024 15:36

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label May 22, 2024

colifran self-assigned this May 22, 2024

This was referenced May 27, 2024

Weekly PR metrics report #30348

Closed

Monthly PR metrics report #30414

Closed

NOUIY mentioned this pull request Jun 14, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#103

Open

snyk-io bot mentioned this pull request Jun 20, 2024

[Snyk] Upgrade aws-cdk-lib from 2.138.0 to 2.143.1 Opetushallitus/heratepalvelu#280

Open

NOUIY mentioned this pull request Jun 20, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#104

Open

snyk-io bot mentioned this pull request Jun 21, 2024

[Snyk] Upgrade aws-cdk-lib from 2.139.0 to 2.143.1 Opetushallitus/ovara#36

Open

rwlxxvii mentioned this pull request Jun 22, 2024

[Snyk] Upgrade aws-cdk-lib from 2.139.0 to 2.144.0 rwlxxvii/containers#140

Merged

This was referenced Jun 22, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#105

Open

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#106

Open

This was referenced Jul 5, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#107

Open

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#108

Open

This was referenced Jul 16, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#109

Open

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#110

Open

NOUIY mentioned this pull request Jul 23, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#111

Open

aws locked as resolved and limited conversation to collaborators Jul 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(stepfunctions-tasks): runtime language used to evaluate expressions is ignored #30302

fix(stepfunctions-tasks): runtime language used to evaluate expressions is ignored #30302

colifran commented May 22, 2024 •

edited

Loading

aws-cdk-automation left a comment

aws-cdk-automation left a comment

aws-cdk-automation commented May 22, 2024

bergjaak May 22, 2024

bergjaak May 22, 2024

colifran May 22, 2024

mergify bot commented May 22, 2024

aws-cdk-automation commented Jul 25, 2024

fix(stepfunctions-tasks): runtime language used to evaluate expressions is ignored #30302

fix(stepfunctions-tasks): runtime language used to evaluate expressions is ignored #30302

Conversation

colifran commented May 22, 2024 • edited Loading

Reason for this change

Description of changes

Description of how you validated changes

Checklist

aws-cdk-automation left a comment

Choose a reason for hiding this comment

aws-cdk-automation left a comment

Choose a reason for hiding this comment

aws-cdk-automation commented May 22, 2024

AWS CodeBuild CI Report

bergjaak May 22, 2024

Choose a reason for hiding this comment

bergjaak May 22, 2024

Choose a reason for hiding this comment

colifran May 22, 2024

Choose a reason for hiding this comment

mergify bot commented May 22, 2024

aws-cdk-automation commented Jul 25, 2024

colifran commented May 22, 2024 •

edited

Loading