fix: avoid logging the same resolved layer multiple times #33
Conversation
index.js
Outdated
| @@ -199,6 +200,10 @@ class ServerlessPlugin { | |||
| args.unshift(TAG); | |||
| } | |||
|
|
|||
| // Avoid logging the same resolved layer multiple times | |||
There was a problem hiding this comment.
Please don't place business logic for specific use-case here. Think log method as console.log. This method should be for general logging. Move this to replaceLayerVersions method implementation instead.
There was a problem hiding this comment.
- declare set which stores "resolved" layer arn
- if set does not contain given layer arn, log & store given layer arn, otherwise, just skip.
and It would be appreciated if you would implement "log level" option.
There was a problem hiding this comment.
Hey, I've updated the PR.
I just didn't do anything about "log level" because I didn't quite get what this option would do.
There was a problem hiding this comment.
I thought we can implement a plugin option for controlling log output verbosity.
For example,
- INFO: Most verbose logging output. Prints any informational log (e.g. prints entire layer replacements without deduplication, with entire context including function name and layer arn)
- WARN: Prints warnings only. Prints warnings only (e.g. prints deduplicated layer replacements - which current PR does)
- ERROR: Prints errors only. Prints errors only (e.g. unknown layer arn)
Then, Users can control log verbosity like this:
// serverless.yml
// ... (truncated)
plugins:
- serverless-latest-layer-version
custom:
latestLayerVersion:
logLevel: WARNAnyway this isn't mandatory for merging. You can ignore this request ;)
There was a problem hiding this comment.
I will happily ignore this request, but I think this would be a great addition to this plugin. I've created a new issue for this feature: #34.
There was a problem hiding this comment.
Thank you for creating an issue for that 👍
index.js
Outdated
| @@ -12,6 +12,7 @@ class ServerlessPlugin { | |||
| this.serverless = serverless; | |||
| this.provider = serverless.getProvider("aws"); | |||
| this.options = options; | |||
| this.resolvedLayers = []; | |||
There was a problem hiding this comment.
I prefer Set over Array to store unique items.
| this.resolvedLayers = []; | |
| this.resolvedLayerSet = new Set(); |
index.js
Outdated
| @@ -181,6 +182,9 @@ class ServerlessPlugin { | |||
| const resolvedLayerArn = arnVersionMap.get(node); | |||
| if (resolvedLayerArn) { | |||
| this.update(resolvedLayerArn); | |||
| // Avoid logging the same resolved layer multiple times | |||
| if (self.resolvedLayers.some(item => item === resolvedLayerArn)) return | |||
There was a problem hiding this comment.
Then this can be expressed as more cleaner way :)
| if (self.resolvedLayers.some(item => item === resolvedLayerArn)) return | |
| if (self.resolvedLayers.has(resolvedLayerArn)) { | |
| return; | |
| } | |
| self.resolvedLayers.add(resolvedLayerArn); |
|
Just merged this PR and new release will be automatically published to npm. Stay tuned! |
## [2.1.2](v2.1.1...v2.1.2) (2022-11-08) ### Bug Fixes * avoid logging the same resolved layer multiple times ([#33](#33)) ([33f289f](33f289f))
|
🎉 This PR is included in version 2.1.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
Okay, v2.1.2 just published to npm. Please let me know that release fixes your issue (or if you need anything further). |
In my use case, I have about 30 lambdas and each one of them have the same 2 layers, which makes the terminal look like this on every deploy:
This small fix caches the resolved layers and only log once for each one, making it look a little more pleasent.