-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: stage assets under .cdk.assets #2182
Conversation
To ensure that assets are available for the toolchain to deploy after the CDK app exists, the CLI will, by default, request that the app will stage the assets under the `.cdk.assets` directory (relative to working directory). The CDK will then *copy* all assets from their source locations to this staging directory and will refer to the staging location as the asset path. Assets will be stored using their content fingerprint (md5 hash) so they will never be copied twice unless they change. Docker build context directories will also be staged. Staging is disabled by default and in cdk-integ. Added .cdk.staging to all .gitignore files in cdk init templates. Fixes #1716
96aa88b
to
e7ba9e0
Compare
Does that mean the tempdir will go in the user's app directory? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't care for the default location of the staging directory. I think it's going to invite more questions and discussions. But it'll be easy enough to change, so okay.
By the way, this PR could be trivially changed to also fix #2096, by having assets.Staging
output the relative file path and the assets handers in the toolkit resolving directories relative to the staging root.
Can you elaborate what you don’t care about?
It actually fixes this already. Added. |
First, everyone who has a CDK project today will not have this directory in their Second, the directory is not going to be as inconspicuous as you think on Windows. I'd prefer for this directory to be off to the side somewhere, maybe in $TMP |
How about $HOME? This way, assets are reused across CDK apps. |
If we use a stable name under $TMP we can also reuse across CDK apps, and I think putting there covers the intent better. Or it would have to be I think Big downside is that if we go for an out-of-scope directory, we're on the hook for periodically cleaning it out as well. Which in fact, we should be doing for an in-scope directory as well anyway. |
We use stable names (based off of the content fingerprint). It is a short of cache. I am leaning towards leaving this under the project directory though as it aligns better with our longer term Toolchain project where “synth” and “bundle” happen on different machines so we need to move all the asset sources between those machines. |
In that case, are the default settings maybe optimizing for CI builds, thereby accidentally pessimizing local builds? If so, we could switch the default off of the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. The hashing and copying logic will be super useful for the toolkit changes.
@rix0rrr decided for now to leave it under the project directory. @sam-goodwin is going to refactor this quite a bit in his work on the toolchain and I rather keep this simple for now. |
…2027) This Code type is helpful when there is no local file/directory to use Assets with, and the Lambda is supposed to be deployed in a CodePipeline.
Until aws/jsii#442 is released we must fail builds on compilation errors. This is a temporary workaround for this issue (if there is something in stdout we consider that a compilation error).
Thanks so much for taking the time to contribute to the AWS CDK ❤️ We will shortly assign someone to review this pull request and help get it
|
To ensure that assets are available for the toolchain to deploy after the CDK app
exists, the CLI will, by default, request that the app will stage the assets under
the
.cdk.assets
directory (relative to working directory).The CDK will then copy all assets from their source locations to this staging
directory and will refer to the staging location as the asset path. Assets will
be stored using their content fingerprint (md5 hash) so they will never be copied
twice unless they change.
Docker build context directories will also be staged.
Staging is disabled by default and in cdk-integ.
Added .cdk.staging to all .gitignore files in cdk init templates.
Fixes #1716
Fixes #2096
Pull Request Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.