fix(stepfunctions-tasks): batch properties do not accept JsonPaths

[peterwoodworth]

fixes #19546

---

All Submissions:

• Have you followed the guidelines in our Contributing guide?

Adding new Unconventional Dependencies:

• This PR adds new unconventional dependencies following the process described here

New Features

• Have you added the new feature to an integration test?
  • Did you use cdk-integ to deploy the infrastructure and generate the snapshot (i.e. cdk-integ without --dry-run)?

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

[gitpod-io]

[kaizencc]

@rix0rrr, I'm a bit confused about the necessity for something like this. Originally we had

Value: `${maybeToken}`

But this does not work when maybeToken is a token. Not sure why this is the case. I looked for prior art for something like this and couldn't find anything. The typical Lazy.number({ produce: () => {}}) isn't applicable because the token is a JsonPath (and there's some fancy footwork that goes on behind the scenes for JsonPaths).

[kaizencc]

@peterwoodworth I changed this up rather than review because I knew something was off but couldn't put my finger on it until I changed things around. Previously, you had:

Value: Token.isUnresolved(containerOverrides.memory) ? containerOverrides.memory: `${containerOverrides.memory.toMebibytes()}`

This failed when I tested whether the memory property supported tokenized values, which was omitted before.

My thoughts on why this is the case: From the isUnresolved docs:

/**
   * Returns true if obj represents an unresolved value
   *
   * One of these must be true:
   *
   * - `obj` is an IResolvable
   * - `obj` is a string containing at least one encoded `IResolvable`
   * - `obj` is either an encoded number or list
   *
   * This does NOT recurse into lists or objects to see if they
   * containing resolvables.
   *
   * @param obj The object to test.
   */

Since a user would submit something like cdk.Size(sfn.JsonPath.numberAt('$.whatever')), Token.isUnresolved(containerOverrides.memory) would check for the resolvability of the Size object, and not the properties of the Size object. Instead, we want to see inside cdk.Size, which is why we want to check for: Token.isUnresolved(containerOverrides.memory.toMebibytes()). Hope that makes sense. Maybe @rix0rrr can chime in if what I say is wrong or not the full story.

[kaizencc]

Related is #19569, which adds the isUnresolved to Size.

[rix0rrr]

Related is also #19578 which fixes this whole class of errors once and for all

[kaizencc]

I notice that we're doing something problematic here as well. props.timeout.toSeconds() is not intended to succeed with Tokens:

function convert(amount: number, fromUnit: TimeUnit, toUnit: TimeUnit, { integral = true }: TimeConversionOptions) {
  if (fromUnit.inMillis === toUnit.inMillis) { return amount; }

  if (Token.isUnresolved(amount)) {
    throw new Error(`Unable to perform time unit conversion on un-resolved token ${amount}.`);
  }
  ...
}

However it will succeed if the user specified their tokenized timeout in seconds as well. So

timeout: Duration.seconds(JsonPath.numberAt('$.whatever'));

has no error while

timeout: Duration.minutes(JsonPath.numberAt('$.whatever'));

will fail with an unintelligible error message.

It makes more sense to use the duration.isUnresolved() function and only assert on non-tokens.

Now, I understand that timeout must be in seconds so sending Duration.minutes(..) is wrong. However, I think the error message 'unable to perform time unit conversion on un-resolved token' does not help the user understand what the error is in this case.

[rix0rrr]

We can change the error message to something like:

Duration must be specified in 'seconds()' here if its value comes from a token

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
• Commit ID: 7437946
• Result: FAILED
• Build Logs (available for 30 days)

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