feat(synthetics): add Python runtime and latest Nodejs runtime#16069
feat(synthetics): add Python runtime and latest Nodejs runtime#16069mergify[bot] merged 13 commits intoaws:masterfrom kaizencc:syn-runtime-2
Conversation
|
|
| /** | ||
| * All known Lambda runtime families. | ||
| */ | ||
| export enum RuntimeFamily { |
There was a problem hiding this comment.
let's also make this a class with static properties. as it stands, customers could not unblock themselves to add a new runtime if the runtime used a family we did not support
There was a problem hiding this comment.
+1 for the end goal of customers unblocking themselves. I think that what makes the most sense is to keep this as an enum and add an OTHER property. Updated the code to reflect this.
There was a problem hiding this comment.
I'm not sure if I'm a huge fan of adding a property that currently doesn't have any meaning. It feels to me that the class with static properties is usually how we deal with this, but I also understand that the actual value that would be supplied to the constructor of that class wouldn't actually be used there. Your choice!
There was a problem hiding this comment.
Does it help that the lambda module does this similarly?
aws-cdk/packages/@aws-cdk/aws-lambda/lib/runtime.ts
Lines 23 to 31 in 62e2f19
I feel like it is odd to have to specify a new RuntimeFamily('java') when the Runtime Family doesn't really hold any meaning in cloudformation, it is just used to check for errors (at the moment). I feel like users will have a hard time knowing what to put for the new runtime family since they could literally write anything. There would be no difference in new RuntimeFamily('java') and new RuntimeFamily('ruby').
It seems cleaner to just have users specify a RuntimeFamily.OTHER in this case. It is basically a flag telling the user that they lose all CDK error checking functionality.
packages/@aws-cdk/aws-synthetics/test/canaries/python/canary.py
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/aws-synthetics/test/canaries/python/canary.py
Outdated
Show resolved
Hide resolved
BenChaimberg
left a comment
There was a problem hiding this comment.
See comments on previous review
|
Should be gtg unless you feel strongly about the |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
…6069) This PR addresses the fact that the current synthetics module was built to support nodejs runtimes only by opening support for python runtimes. Closes aws#15138 and aws#16177. **Breaking Changes** - `Runtime('customRuntimeHere')` becomes `Runtime('customRuntime', 'runtimeFamily')` - `Code.fromAnything('path').bind(this, 'handler')` becomes `Code.fromAnything('path').bind(this, 'handler', 'runtimeFamily')` **Whats in this PR?** - Adds latest Nodejs runtime (`syn-nodejs-puppeteer-3.2`) and updates integ test to it. - Adds generic python script to the folder `test/canaries/python` in order to run unit & integration tests on it. - Adds new `RuntimeFamily` enum that is required by the `Runtime` object to differentiate between Python and Node. - Verifies the correct folder structure for Python runtimes (`python/<filename>.py`). - Updates readme. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…6069) This PR addresses the fact that the current synthetics module was built to support nodejs runtimes only by opening support for python runtimes. Closes aws#15138 and aws#16177. **Breaking Changes** - `Runtime('customRuntimeHere')` becomes `Runtime('customRuntime', 'runtimeFamily')` - `Code.fromAnything('path').bind(this, 'handler')` becomes `Code.fromAnything('path').bind(this, 'handler', 'runtimeFamily')` **Whats in this PR?** - Adds latest Nodejs runtime (`syn-nodejs-puppeteer-3.2`) and updates integ test to it. - Adds generic python script to the folder `test/canaries/python` in order to run unit & integration tests on it. - Adds new `RuntimeFamily` enum that is required by the `Runtime` object to differentiate between Python and Node. - Verifies the correct folder structure for Python runtimes (`python/<filename>.py`). - Updates readme. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
This PR addresses the fact that the current synthetics module was built to support nodejs runtimes only by opening support for python runtimes. Closes #15138 and #16177.
Breaking Changes
Runtime('customRuntimeHere')becomesRuntime('customRuntime', 'runtimeFamily')Code.fromAnything('path').bind(this, 'handler')becomesCode.fromAnything('path').bind(this, 'handler', 'runtimeFamily')Whats in this PR?
syn-nodejs-puppeteer-3.2) and updates integ test to it.test/canaries/pythonin order to run unit & integration tests on it.RuntimeFamilyenum that is required by theRuntimeobject to differentiate between Python and Node.python/<filename>.py).By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license