fix(lambda-nodejs): logLevel property of BundlingOptions is ignored when nodeModules are defined#18456
Conversation
|
|
logLevel property of BundlingOptions is ignored when nodeModules are definedlogLevel property of BundlingOptions is ignored when nodeModules are defined
|
Title does not follow the guidelines of Conventional Commits. Please adjust title before merge. |
logLevel property of BundlingOptions is ignored when nodeModules are definedlogLevel property of BundlingOptions is ignored when nodeModules are defined
|
@jogold can you take a look? |
Will do, this week or next week. |
|
@jogold did you have a chance to take a look at this? |
jogold
left a comment
There was a problem hiding this comment.
Sorry for the late reply @flavioleggio.
This looks good.
Can you update the jsdoc in types.ts both for the LogLevel enum and the logLevel prop to reflect that this log level doesn't only apply to esbuild now?
|
Hi @jogold
No worries!
Done |
|
@corymhall LGTM |
|
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). |
…ed when `nodeModules` are defined (aws#18456) In this pull request I managed to propagate the effect of the `logLevel` property downstream to the `PackageManager` class. I had to refactor the static approach for constructing handler objects for the various package managers as we need to pass a new optional property in the `fromLockFile(lockFilePath: string)` static builder method. The value for `installCommand` attribute in the constructor now depends on the log level and the approach varies between package managers: while `npm ci` supports a flexible `--loglevel` command option matching each of the `LogLevel` enum existing values, `yarn install` only supports a `--silent` option and similarly `pnpm install` supports a `--reporter=silent` which suppresses the command output except for errors and warnings. For `yarn` and `pnpm` my idea is to include the silent options whenever a log level is specified and its value is different from the default `LogLevel.INFO`. I also adapted existing unit tests and added new ones taking into account the property's value. I have no experience with yarn and pnpm and I am not sure that any tests exist using bundling with yarn- and pnpm-based lambda function handlers, so I cannot guarantee that my changes work for that too. Fixes aws#18383 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
In this pull request I managed to propagate the effect of the
logLevelproperty downstream to thePackageManagerclass.I had to refactor the static approach for constructing handler objects for the various package managers as we need to pass a new optional property in the
fromLockFile(lockFilePath: string)static builder method.The value for
installCommandattribute in the constructor now depends on the log level and the approach varies between package managers: whilenpm cisupports a flexible--loglevelcommand option matching each of theLogLevelenum existing values,yarn installonly supports a--silentoption and similarlypnpm installsupports a--reporter=silentwhich suppresses the command output except for errors and warnings. Foryarnandpnpmmy idea is to include the silent options whenever a log level is specified and its value is different from the defaultLogLevel.INFO.I also adapted existing unit tests and added new ones taking into account the property's value. I have no experience with yarn and pnpm and I am not sure that any tests exist using bundling with yarn- and pnpm-based lambda function handlers, so I cannot guarantee that my changes work for that too.
Fixes #18383
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license