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

feat(core): customize bundling output packaging #13076

Merged
merged 20 commits into from
Feb 17, 2021
Merged

Conversation

jogold
Copy link
Contributor

@jogold jogold commented Feb 16, 2021

If the bundling output contains a single archive file (zip or jar), upload it
as-is to S3 without zipping it.

Allow to customize this behavior with bundling.packaging:

  • ALWAYS_ZIP: The bundling output will always be zipped and uploaded to S3.
  • NEVER_ZIP: The bundling output will not be zipped. Bundling will fail if
    the bundling output doesn't contain a single file.
  • AUTO: If the bundling output contains a single archive file (zip or jar) it
    will not be zipped. Otherwise it will be zipped.

Closes #10776
Closes #12651


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

@gitpod-io
Copy link

gitpod-io bot commented Feb 16, 2021

@jogold jogold changed the title feat(core): custom archive with bundling feat(core): customize bundling output packaging Feb 16, 2021
@jogold jogold marked this pull request as ready for review February 16, 2021 15:38
@jogold
Copy link
Contributor Author

jogold commented Feb 16, 2021

@eladb if you're going to take this one, this is ready for review.

Copy link
Contributor

@eladb eladb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rix0rrr take a look also please.

packages/@aws-cdk/aws-s3-assets/README.md Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-s3-assets/README.md Outdated Show resolved Hide resolved
packages/@aws-cdk/core/lib/asset-staging.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/core/lib/asset-staging.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/core/lib/asset-staging.ts Show resolved Hide resolved
packages/@aws-cdk/core/lib/asset-staging.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/core/lib/bundling.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/core/lib/bundling.ts Outdated Show resolved Hide resolved
jogold and others added 4 commits February 17, 2021 09:50
Co-authored-by: Elad Ben-Israel <benisrae@amazon.com>
Co-authored-by: Elad Ben-Israel <benisrae@amazon.com>
@jogold jogold requested a review from eladb February 17, 2021 13:35
eladb
eladb previously approved these changes Feb 17, 2021
packages/@aws-cdk/aws-s3-assets/README.md Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-s3-assets/README.md Outdated Show resolved Hide resolved
packages/@aws-cdk/core/lib/asset-staging.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/core/lib/bundling.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/core/lib/bundling.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/core/lib/bundling.ts Outdated Show resolved Hide resolved
@mergify
Copy link
Contributor

mergify bot commented Feb 17, 2021

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

jogold and others added 2 commits February 17, 2021 16:27
Co-authored-by: Elad Ben-Israel <benisrae@amazon.com>
Co-authored-by: Elad Ben-Israel <benisrae@amazon.com>
@mergify mergify bot dismissed eladb’s stale review February 17, 2021 15:29

Pull request has been modified.

jogold and others added 4 commits February 17, 2021 16:29
Co-authored-by: Elad Ben-Israel <benisrae@amazon.com>
Co-authored-by: Elad Ben-Israel <benisrae@amazon.com>
eladb
eladb previously requested changes Feb 17, 2021
Copy link
Contributor

@eladb eladb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor

packages/@aws-cdk/core/lib/bundling.ts Outdated Show resolved Hide resolved
@mergify mergify bot dismissed eladb’s stale review February 17, 2021 16:30

Pull request has been modified.

eladb
eladb previously approved these changes Feb 17, 2021
@mergify mergify bot dismissed eladb’s stale review February 17, 2021 16:35

Pull request has been modified.

@iliapolo
Copy link
Contributor

Nice, this also addresses #12651

@mergify
Copy link
Contributor

mergify bot commented Feb 17, 2021

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
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: 14a0ad1
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@mergify
Copy link
Contributor

mergify bot commented Feb 17, 2021

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

@mergify mergify bot merged commit 367a055 into aws:master Feb 17, 2021
NovakGu pushed a commit to NovakGu/aws-cdk that referenced this pull request Feb 18, 2021
If the bundling output contains a single archive file (zip or jar), upload it
as-is to S3 without zipping it.

Allow to customize this behavior with `bundling.packaging`:
* `ALWAYS_ZIP`: The bundling output will always be zipped and uploaded to S3.
* `NEVER_ZIP`: The bundling output will not be zipped. Bundling will fail if
  the bundling output doesn't contain a single file.
* `AUTO`: If the bundling output contains a single archive file (zip or jar) it
  will not be zipped. Otherwise it will be zipped.

Closes aws#10776 
Closes aws#12651

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@NetaNir
Copy link
Contributor

NetaNir commented Feb 19, 2021

Im fairly convinced this change is the root cause of #13131. The new code ignores already zipped files and tries to re zip them, which is throwing an error.

cc @jogold

@NetaNir
Copy link
Contributor

NetaNir commented Feb 19, 2021

Give the large impact Im going to revert this change and release a patch.

@eladb
Copy link
Contributor

eladb commented Feb 19, 2021

Please revert and we will look into it next week.

@jogold jogold deleted the single-archive branch February 19, 2021 06:44
eladb pushed a commit that referenced this pull request Feb 19, 2021
This commit reverts two recent changes to the asset system  (#12258 and ##13076) which introduced a regression in 1.90.0.

Fixes #13131
eladb pushed a commit that referenced this pull request Feb 19, 2021
This commit reverts two recent changes to the asset system  (#12258 and ##13076) which introduced a regression in 1.90.0.

Fixes #13131
mergify bot pushed a commit that referenced this pull request Feb 19, 2021
This commit reverts two recent changes to the asset system  (#12258 and ##13076) which introduced a regression in 1.90.0.

Fixes #13131


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
NetaNir pushed a commit that referenced this pull request Feb 19, 2021
This commit reverts two recent changes to the asset system  (#12258 and ##13076) which introduced a regression in 1.90.0.

Fixes #13131

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@jogold jogold restored the single-archive branch February 19, 2021 09:13
eladb pushed a commit that referenced this pull request Feb 22, 2021
If the bundling output contains a single archive file (zip or jar), upload it
as-is to S3 without zipping it.

Allow to customize this behavior with `bundling.packaging`:
* `ALWAYS_ZIP`: The bundling output will always be zipped and uploaded to S3.
* `NEVER_ZIP`: The bundling output will not be zipped. Bundling will fail if
  the bundling output doesn't contain a single file.
* `AUTO`: If the bundling output contains a single archive file (zip or jar) it
  will not be zipped. Otherwise it will be zipped.

Closes #10776 
Closes #12651

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
eladb pushed a commit that referenced this pull request Feb 22, 2021
This commit reverts two recent changes to the asset system  (#12258 and ##13076) which introduced a regression in 1.90.0.

Fixes #13131

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
eladb pushed a commit that referenced this pull request Feb 22, 2021
This commit reverts two recent changes to the asset system  (#12258 and ##13076) which introduced a regression in 1.90.0.

Fixes #13131


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
mergify bot pushed a commit that referenced this pull request Feb 28, 2021
Redo of #13076 after #13131. The fix is [`7b3d829` (#13152)](7b3d829).

If the bundling output contains a single archive file (zip or jar), upload it
as-is to S3 without zipping it.

Allow to customize this behavior with `bundling.outputType`:
* `NOT_ARCHIVED`: The bundling output will always be zipped and uploaded to S3.
* `ARCHIVED`: The bundling output will not be zipped. Bundling will fail if
  the bundling output doesn't contain a single archive file.
* `AUTO_DISCOVER`: If the bundling output contains a single archive file (zip or jar) it
  will not be zipped. Otherwise it will be zipped. This is the default.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/core Related to core CDK functionality
Projects
None yet
6 participants