chore(core): allow the bundler to re-use pre-existing bundler output (revisited)

[misterjoshua]

This PR changes AssetStaging so that the bundler will re-use pre-existing output. Before, the bundler would re-run Docker without considering pre-existing assets, which was slow. Now, when handling a SOURCE hash type, the bundler detects and returns pre-existing asset output without re-running Docker. For all other hash types, the bundler outputs to an intermediate directory before calculating asset hashes, then renames the intermediate directory into its final location.

This PR revisits #8916 which originally closed #8882. Here are some details from the previous PR which have been addressed in this PR:

• The bundler now outputs directly into the assembly directory
• The bundler's assets can be reused between multiple syntheses
• The bundler keeps output from failed bundling attempts for diagnosability purposes (renamed with an -error suffix)
• Bundler options are hashed together with custom and source hashes
• Removed the check for a docker run from throws with assetHash and not CUSTOM hash type as docker is no longer run before the AssetStaging props are validated.

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[mergify]

Title does not follow the guidelines of Conventional Commits. Please adjust title before merge.

[misterjoshua]

@jogold Here's that PR for the progress I promised this afternoon. It probably looks familiar in a lot of ways. :)

[adamdottv]

@misterjoshua I think this will close #9406? cc @jogold

[misterjoshua]

@misterjoshua I think this will close #9406? cc @jogold

This PR will help to speed up the creation of separate assets and lambda layers that might be bundled with pip install, but I don't think this PR will close #9406 outright. We need some way to separate the assets. Otherwise, any small changes to the lambda's code will change the source hash and re-signal a complete pip install.

After this PR is merged, I imagine that it would be possible to change PythonFunction so that it does pip install in a separate lambda layer if it detects the requirements.txt. But, we should attack this in a different PR.

[jogold]

This looks good! Can you merge the latest changes from master? Local bundling has been added.

[misterjoshua]

@jogold I've merged changes from master and resolved a small conflict. CI is green.

I'm looking over the /// !cdk-integ pragma:ignore-assets stuff right now - these pragmas aren't sitting well with me. I'm going to see about updating the expectations in these files rather than using the pragma.

[misterjoshua]

@jogold I've merged changes from master and resolved a small conflict. CI is green.

I'm looking over the /// !cdk-integ pragma:ignore-assets stuff right now - these pragmas aren't sitting well with me. I'm going to see about updating the expectations in these files rather than using the pragma.

It looks like there's interesting behaviour surrounding /// !cdk-integ pragma:ignore-assets in integ.assets.bundling.lit.ts. The asset hashes are unstable in this test. The image is being built on-demand, so timestamps and the Docker layer cache are in play for the resulting Image ID. I'm working on a solution to this problem now.

Pull request has been modified.

[mergify]

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-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
• Commit ID: 56b351e
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[mergify]

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).

[misterjoshua]

@eladb I see that in master, this PR's associated commit has a red X for failing build. Is there anything I need to do to help?

[BryanPan342]

seems like the the error is in aws-logs.. i have a PR that's failing after merging master too

[BryanPan342]

specifically

lib/log-retention-provider/index.ts:6:10 - error TS2305: Module '"../../node_modules/aws-sdk/lib/config"' has no exported member 'RetryDelayOptions'.

[misterjoshua]

specifically

lib/log-retention-provider/index.ts:6:10 - error TS2305: Module '"../../node_modules/aws-sdk/lib/config"' has no exported member 'RetryDelayOptions'.

@BryanPan342 I see that as well, but while building locally - this is new. I'm not sure how this PR could have impacted that. AWS SDK just released a new version, however, and TypeScript types were referenced. I wonder if this is related?

bugfix: TypeScript type definitions: Deep imports of clients no longer reference entire SDK type definitions

[BryanPan342]

#9994 i updated the dependency it seems to work locally now but we shall see what happens with code-build runs it

[BryanPan342]

ah @misterjoshua i think ur right

https://github.com/aws/aws-sdk-js/blob/9bd7fdbda9929d4f259996d468de80e210a4ce78/lib/config-base.d.ts

they moved the RetryDelayOptions to config-base and had the config class extend it but im assuming the versioning must messed up somewhere