@nicholaschiasson The integ test aws-route53/test/integ.record-weight-from-cfnparameter failed, would you be able to fix that and I'm happy to approve again to get this merged in.

@GavinZZ thanks! The integ test is now fixed.

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).

Hey @GavinZZ , sorry, I had misunderstood the first time around why the integration test was unstable, but I am confident I have solved it now. The issue was related to how the set identifier was derived if one was not provided with constructor options.

I realized that when generating a set ID, the method restricts the length of the ID to 64 characters. Since token.valueAsNumber produces a very long and non-deterministic-length (stringified) number, the test could now fail randomly. (Eg. -1.8881545897089744e+289)

To solve this, I modified the configureSetIdentifier method to check if the weight property is a token, and in that case, essentially mock the weight value as a three character string, since we can't know the parameter value at synthesis time, but we know that the maximum value allowed for weight is 255. After generating the identifier, I inject the Token as string instead of as number, just so it may be more helpful for a developer maybe for debugging sake (Eg. ${Token[TOKEN.1037]}). Regardless of if it is asNumber or asString, this will be substituted during synthesis. During CDK runtime however, maybe it could be helpful.

Let me know if this is a satisfactory solution. At the very least, the integ test should now be stable.

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).

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).

Hi @GavinZZ , it seems like something is blocking the merge. Do you have any detail about that?

AWS CodeBuild CI Report

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

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

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).

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.