terraform-cdk icon indicating copy to clipboard operation
terraform-cdk copied to clipboard

Do not overwrite all `NODE_OPTIONS`

Open nikolay opened this issue 1 year ago • 3 comments

https://github.com/hashicorp/terraform-cdk/blob/main/packages/cdktf-cli/lib/synth-stack.ts#L72=

The logic is broken as, first, this overrides all settings and, second, it warns without checking if this settings is already at the minimum required value!

nikolay avatar Aug 08 '22 22:08 nikolay

Also, isn't the value supposed to be 4096?

nikolay avatar Aug 08 '22 22:08 nikolay

Last, but not least - the style of warning should change to be consistent:

$ npm run synth

> [REDACTED]@0.1.0 synth

found NODE_OPTIONS environment variable without a setting for --max-old-space-size.
The synthesizing step for TypeScript may need an increased amount of memory if multiple large providers
are used with locally generated bindings. You can ignore this if you don't use CDKTF with TypeScript.
If not present, the cdktf-cli sets it to NODE_OPTIONS="--max-old-space-size=4048" by default. But as
your environment already contains a NODE_OPTIONS variable, we won't override it. Hence, the app command
might fail while synthesizing with an out of memory error.

Generated Terraform code for the stacks: [REDACTED]
$ npm run get

> [REDACTED]@0.1.0 get

[2022-08-08T15:14:14.672] [WARN] default - WARNING: No providers or modules found in "cdktf.json" config file, therefore cdktf get does nothing.

nikolay avatar Aug 08 '22 22:08 nikolay

Also, isn't the value supposed to be 4096?

Hehe, yeah. Node does not care, but 4096 would be the right value 😅

ansgarm avatar Aug 09 '22 09:08 ansgarm

I'm going to lock this issue because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

github-actions[bot] avatar Nov 25 '22 01:11 github-actions[bot]