chore(build): fast fail PR builds if rosetta extraction fails #14946
Conversation
|
|
…nto chaimber/fast_fail_extract
|
Adding extraction increases the PR build time by about 9 minutes (25 min -> 34 min) |
| @@ -35,7 +40,7 @@ while [[ "${1:-}" != "" ]]; do | |||
| done | |||
|
|
|||
| export PATH=$(npm bin):$PATH | |||
| export NODE_OPTIONS="--max-old-space-size=4096 ${NODE_OPTIONS:-}" | |||
| export NODE_OPTIONS="--max-old-space-size=4096 --experimental-worker ${NODE_OPTIONS:-}" | |||
There was a problem hiding this comment.
We probably need to be a bit careful here.
I believe this option is required only for Node 10. In later versions of node, this has graduated out of experimental. And not supported in earlier versions of node.
What's the experience when this option is set for version > node 10? Fail or ignore?
We could be super precise and say - if node 10 and snippets extraction is set, then use --experiemental-worker and if < node10 and snippet extraction is set, fail?
WDYT? too pedantic?
There was a problem hiding this comment.
The following is just speculation on how node should behave since I haven't been able to easily find node's policies on experimental features.
Providing flags that enable experimental features after the feature has stabilized should not have any effect on behaviour (confirmed for v10, v12, and v14). Further, the flag should not be reused for another purpose. Any other experience would be very frustrating to customers, forcing them to configure different versions in different ways and breaking legacy scripts.
So I don't think it's necessary to be careful with adding this flag. I'm of course happy to do so if you feel otherwise!
I also don't think we should fail if the node version is < 10, since rosetta itself still supports single-threaded extraction (though it is quite slow, especially for the monolithic modules). Rosetta already emits a warning in this case.
| @@ -14,4 +14,4 @@ phases: | |||
| - yarn --version || npm -g install yarn | |||
| build: | |||
| commands: | |||
| - /bin/bash ./build.sh && git diff-index --exit-code --ignore-space-at-eol --stat HEAD | |||
| - /bin/bash ./build.sh --extract && git diff-index --exit-code --ignore-space-at-eol --stat HEAD | |||
There was a problem hiding this comment.
Out of curiosity, how many packages do we currently have rosetta extract enabled?
And what is the cost of this extra work on our build time? The PR build for this PR should give you that, baselined against any other PR build.
There was a problem hiding this comment.
203 + monocdk + aws-cdk-lib = 205 have an extraction script. It doesn't look like any of the packages have strict extraction enabled currently.
As I commented above, it seems to be about a 9 minute increase in build time. Not horrible but also not great. From a cursory investigation, it seems to take <50ms on my local machine to bring each worker online, so we don't seem to be losing much to overhead subprocess costs. I can do a deeper dive on how to speed it up if you think it's warranted.
|
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). |
|
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). |
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). |
) Modify the build script to run jsii-rosetta extraction if the `--extract` flag is passed. This will allow PR builds to run extraction, ensuring that modules who have strict snippet compilation enabled will fail to build if a change causes a snippet to fail compilation. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Modify the build script to run jsii-rosetta extraction if the
--extractflag is passed. This will allow PR builds to run extraction, ensuring that modules who have strict snippet compilation enabled will fail to build if a change causes a snippet to fail compilation.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license