Skip to content
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

[@aws-cdk/aws-events] Schedule cannot be created from CfnParameter #9413

Closed
alopix opened this issue Aug 3, 2020 · 6 comments
Closed

[@aws-cdk/aws-events] Schedule cannot be created from CfnParameter #9413

alopix opened this issue Aug 3, 2020 · 6 comments
Labels
@aws-cdk/aws-events Related to CloudWatch Events effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p2

Comments

@alopix
Copy link

alopix commented Aug 3, 2020

When trying to use a parameter for the duration of a events Schedule, on synth, an error occurs: "Unable to perform time unit conversion on un-resolved token -1.8881545897087682e+289."

Specifying the duration for e.g. a Lambda timeout works, but not if the Duration is used in a Schedule.rate.

Reproduction Steps

import { Stack, App, Duration, CfnParameter } from '@aws-cdk/core';
import { Rule, Schedule } from '@aws-cdk/aws-events';

const invokeScheduleInMinutes = new CfnParameter(this, 'invokeScheduleInMinutes', {
      type: 'Number',
      description: 'The minutes between scheduled lambda invocations.'
});

new Rule(this, 'Rule', {
      ruleName: `cron`,
      schedule: Schedule.rate(Duration.minutes(invokeScheduleInMinutes.valueAsNumber))
});

Error Log

…/node_modules/@aws-cdk/core/lib/duration.ts:289
    throw new Error(`Unable to perform time unit conversion on un-resolved token ${amount}.`);
          ^
Error: Unable to perform time unit conversion on un-resolved token -1.8881545897087682e+289.
    at convert (…/node_modules/@aws-cdk/core/lib/duration.ts:289:11)
    at Duration.toSeconds (…node_modules/@aws-cdk/core/lib/duration.ts:127:12)
    at Function.rate (…/node_modules/@aws-cdk/aws-events/lib/schedule.ts:20:18)
    at new AppStack (…/infrastructure/stacks/src/AppStack.ts:47:26)
    at Object.<anonymous> (…/infrastructure/stacks/src/app.ts:21:1)
    at Module._compile (internal/modules/cjs/loader.js:1158:30)
    at Module.m._compile (…/node_modules/ts-node/src/index.ts:858:23)
    at Module._extensions..js (internal/modules/cjs/loader.js:1178:10)
    at Object.require.extensions.<computed> [as .ts] (…/node_modules/ts-node/src/index.ts:861:12)
    at Module.load (internal/modules/cjs/loader.js:1002:32)

Environment

  • CLI Version : 1.56.0
  • Framework Version: 1.56.0
  • Node.js Version: v12.16.1
  • OS : macOS
  • Language (Version): TypeScript 3.9.7

Other


This is 🐛 Bug Report

@alopix alopix added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Aug 3, 2020
@github-actions github-actions bot added the @aws-cdk/aws-events Related to CloudWatch Events label Aug 3, 2020
@ericzbeard ericzbeard assigned rix0rrr and unassigned ericzbeard Aug 3, 2020
@rix0rrr
Copy link
Contributor

rix0rrr commented Aug 4, 2020

As a workaround, use Schedule.expression() for now.


For fixing this:

Duration should should have an isUnresolved() method, and Schedule.rate should just render the number in the given unit if the duration is unresolved.

Same for other rate() methods in the library.

@rix0rrr rix0rrr added effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p2 and removed bug This issue is a bug. labels Aug 4, 2020
@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label Aug 4, 2020
@SomayaB SomayaB assigned shivlaks and unassigned rix0rrr Aug 20, 2020
@raphael-riel
Copy link
Contributor

For the records, here is a working example for the proposed workaround:

        const rateMinutes = new cdk.CfnParameter(this, "RateMinutes", {
            default: 5,
            description: "...",
            type: "Number",
        });

        new events.Rule(this, "ScheduledFlush", {
            schedule: events.Schedule.expression(`rate(${rateMinutes.value} minutes)`),
            targets: [
                new eventsTargets.LambdaFunction(flushFunction),
            ],
        });

cron(...) expression is also possible. Ref: https://github.com/aws-samples/aws-cdk-examples/blob/master/typescript/lambda-cron/index.ts#L22

@alopix
Copy link
Author

alopix commented Nov 23, 2020

Thanks for the workaround suggestions.
This does not solve the problem though. It will work for the schedule, but e.g. a Metric period is still gonna fail the same way, as it requires a resolved Duration value and does not work from a Parameter.

@NGL321 NGL321 assigned rix0rrr and unassigned shivlaks Jan 25, 2021
mergify bot pushed a commit that referenced this issue Mar 9, 2021
Fix for Issue: #9413

Testing:
Unit tests added for Schedule.rate used in ApplicationAutoScaling and Events.

* [X] Testing
  - Unit test added (prefer not to modify an existing test, otherwise, it's probably a breaking change)
  - __CLI change?:__ coordinate update of integration tests with team
  - __cdk-init template change?:__ coordinated update of integration tests with team
* [X] Docs
  - __jsdocs__: All public APIs documented
  - __README__: README and/or documentation topic updated
  - __Design__: For significant features, design document added to `design` folder
* [X] Title and Description
  - __Change type__: title prefixed with **fix**, **feat** and module name in parents, which will appear in changelog
  - __Title__: use lower-case and doesn't end with a period
  - __Breaking?__: last paragraph: "BREAKING CHANGE: <describe what changed + link for details>"
  - __Issues__: Indicate issues fixed via: "**Fixes #xxx**" or "**Closes #xxx**"
* [ ] Sensitive Modules (requires 2 PR approvers)
  - IAM Policy Document (in @aws-cdk/aws-iam)
  - EC2 Security Groups and ACLs (in @aws-cdk/aws-ec2)
  - Grant APIs (only if not based on official documentation with a reference)
----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@rix0rrr rix0rrr removed their assignment Jun 3, 2021
@jumic
Copy link
Contributor

jumic commented Aug 15, 2021

The code in this issue can be used without error in the meanwhile.
It was resolved in PR #13064. This issue is still open because it wasn't referenced correctly in the PR.

@peterwoodworth Could you please close this issue?

@peterwoodworth
Copy link
Contributor

Will do, thank you

@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-events Related to CloudWatch Events effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p2
Projects
None yet
Development

No branches or pull requests

8 participants