Skip to content
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

(pipelines): make bundling of lambdas in asset stage as container images are done #21848

Open
1 of 2 tasks
Hi-Fi opened this issue Aug 31, 2022 · 5 comments
Open
1 of 2 tasks
Labels
@aws-cdk/pipelines CDK Pipelines library effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. p1

Comments

@Hi-Fi
Copy link
Contributor

Hi-Fi commented Aug 31, 2022

Describe the feature

Currently Lambdas are bundled along the synth, which causes issues for duration of both synth and also tests that test the templates.

If the definition would be created same way as for container images, synth and tests would be quite a lot lighter, and utilization of parallelization in asset stage would benefit users.

Use Case

This change would help user by providing

  • Speed up synth
  • Speed up test runs
  • Prevent need of secrets locally (if bundling requires those, e.g. for access to private registries)

Proposed Solution

Synth should generate same kind of asset definition and copy needed files to cdk.out that's currently done for container based assets.

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

N/A

Environment details (OS name and version, etc.)

N/A

@Hi-Fi Hi-Fi added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Aug 31, 2022
@github-actions github-actions bot added the @aws-cdk/pipelines CDK Pipelines library label Aug 31, 2022
@rix0rrr
Copy link
Contributor

rix0rrr commented Sep 2, 2022

Yep, I agree. I am unhappy with how bundling works currently, and would much prefer it be a discrete preprocessing step.

There is one downside to that: it would be impossible to have the asset hash be the hash of the output artifacts (as opposed to the input artifacts).

image

That means that any nondeterministic commands like apt-get or curl in the bundling command (and remember, it can be arbitrary commands run inside a Docker container) cannot be accurately reflected in the asset hash.

@rix0rrr rix0rrr added effort/large Large work item – several weeks of effort p1 and removed needs-triage This issue or PR still needs to be triaged. labels Sep 2, 2022
@rix0rrr rix0rrr removed their assignment Sep 2, 2022
@Hi-Fi
Copy link
Contributor Author

Hi-Fi commented Sep 2, 2022

@rix0rrr isn't same currently the case with container images? That hash gets precalculated and can produce different outcome in different runs with same hash?

@Hi-Fi
Copy link
Contributor Author

Hi-Fi commented Sep 5, 2022

Probably duplicate with #19157

@Hi-Fi
Copy link
Contributor Author

Hi-Fi commented Sep 17, 2022

Isn't the actual place where this functionality is defined at here?
https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/core/lib/asset-staging.ts#L186-L193

@Hi-Fi
Copy link
Contributor Author

Hi-Fi commented Oct 12, 2022

@rix0rrr Not sure if you checked #22325, but isn't that something that's required to do this bundling happen at separate step? As I don't see that it would be reasonable to start creating any pipelines specific hacks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/pipelines CDK Pipelines library effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. p1
Projects
None yet
Development

No branches or pull requests

2 participants