feat: Reduce aws-cdk-lib package size by not bundling awscli Lambda Layer #21799
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ate flag in package.json so it can be published separately
…at I can write a different implementation for it in v2 (this is a temporary measure, we still need a 'real' way to do make them differnt
… @aws-cdk/lambda-layer-awscli. This currently only works if the dependency is present in node_modules
This does not work, because the subsequent `require()` call fails to load the installed module, because it can't find the aws-cdk-lib and constructs peer-dependencies. Installing both of those seems like a performance cost we do not want to pay.
|
|
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
madeline-k
changed the title
madeline-k
changed the title
rix0rrr
reviewed
Aug 29, 2022
Comment on lines
26
to
31
| const installParentDir = os.homedir() ?? os.tmpdir(); | ||
| this.wellKnownInstallDir = path.join(installParentDir, '.awscdk/npm-cache'); | ||
|
|
||
| const version = this.requireWrapper(path.join(__dirname, '../package.json')).devDependencies[this.packageName]; | ||
| const pathOfModuleIfAlreadyInstalled = require.resolve(`${this.packageName}`); | ||
| const versionAlreadyInstalled = this.requireWrapper(path.join(pathOfModuleIfAlreadyInstalled, '../../package.json')).version; |
There was a problem hiding this comment.
This will ultimately all go into a reusable function/class later, right? (I suppose it could go into core)
rix0rrr
reviewed
Aug 29, 2022
| "constructs": "^10.0.0", | ||
| "lambda-layer-awscli-v1": "0.0.2", |
There was a problem hiding this comment.
Can we get this in the @aws-cdk namespace?
rix0rrr
reviewed
Aug 29, 2022
|
|
||
| private installNpmPackage(): any { | ||
| console.log(`Shelling out to run npm install ${this.packageName} --no-save --prefix ${this.wellKnownInstallDir}`); | ||
| const result = childproc.execSync(`pwd; npm prefix; npm install ${this.packageName} --silent --no-save --prefix ${this.wellKnownInstallDir}`); |
|
Superseded by #21938 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is in draft state, and is not ready to be merged yet. A couple of notes for the reader:
require.main.pathslist. I tried to use a well-known "global" install location, but removed it in this commit because it didn't work.Todos:
All Submissions:
Adding new Unconventional Dependencies:
New Features
yarn integto deploy the infrastructure and generate the snapshot (i.e.yarn integwithout--dry-run)?By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license