feat(sync): add --force-upload flag to control S3 upload dedup#9008
Open
vishwakt wants to merge 1 commit into
Open
feat(sync): add --force-upload flag to control S3 upload dedup#9008vishwakt wants to merge 1 commit into
vishwakt wants to merge 1 commit into
Conversation
`sam sync` previously hardcoded `force_upload=True` in three places, which bypassed the SHA-based dedup in `S3Uploader.upload_with_dedup` and caused every sync to re-upload all artifacts even when nothing had changed. Issue aws#8168 reports a 20%+ (90+ second) speedup from toggling this off in a fork. Expose `--force-upload` on `sam sync` (default False, matching `sam deploy`'s default) and thread it through: - PackageContext and DeployContext, replacing the two hardcoded `True` values in `samcli/commands/sync/command.py`. - The S3Uploader instantiation in ZipFunctionSyncFlow now reads `force_upload` from `self._deploy_context.force_upload` so the flag actually controls the oversized-zip upload path too. Schema regenerated. Unit tests updated to assert the flag flows through PackageContext, DeployContext, and the ZipFunctionSyncFlow S3 upload path for both True and False values. Closes aws#8168
This file contains hidden or 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
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.
Which issue(s) does this change fix?
Closes #8168
Why is this change necessary?
sam synccurrently hardcodesforce_upload=Truein three places. That bypasses the SHA-based dedup insideS3Uploader.upload_with_dedup, so every sync re-zips and re-uploads every artifact to S3 even when nothing has actually changed. The original issue reports a 20%+ (~90s) speedup per run from disabling this on a fork.How does it address the issue?
Exposes
--force-uploadonsam sync, defaulting toFalse(matching the default onsam deployandsam package), and threads the flag through every code path that was previously hardcoded:samcli/commands/sync/command.py— replacesforce_upload=TrueinPackageContext(...)andDeployContext(...)with the new option.samcli/lib/sync/flows/zip_function_sync_flow.py— theS3Uploaderconstructed for oversized lambda zips now readsforce_uploadfromself._deploy_context.force_uploadinstead of hardcodingTrue. This was the third code path that defeated the dedup.force_upload_option()decorator fromsamcli/commands/_utils/options.py, so help text, click metadata, and config-file plumbing are consistent withsam deploy/sam package.schema/samcli.jsonwas regenerated viapython -m schema.make_schemaand now listsforce_uploadas a sync parameter.Users who actually want the old behavior can pass
--force-upload.What side effects does this change have?
--force-upload,sam syncwill now skip uploading artifacts whose SHA already exists in S3 — which is what the issue requester (and the maintainer who welcomed a PR) asked for. Backward-compatible toggle via the new flag.Tests
tests/unit/commands/sync/test_command.py— existing assertions updated fromforce_upload=Truetoforce_upload=self.force_upload, plus a new parameterized test (test_force_upload_flag_propagates_to_package_and_deploy_contexts) covering bothTrueandFalseand asserting the value reaches bothPackageContextandDeployContext.tests/unit/lib/sync/flows/test_zip_function_sync_flow.py— new parameterized test (test_s3_uploader_uses_deploy_context_force_upload) confirming theS3Uploaderis constructed withdeploy_context.force_uploadfor both values.tests/unit/commands/samconfig/test_samconfig.py—do_cliarg-order assertion updated for the new positional parameter.black --check setup.py samcli tests schemaandruff check samcli schemaare both clean locally.Mandatory Checklist
sam deploy/sam package)tests/integration/sync/)make prpasses locally (black --check,ruff check,python -m schema.make_schema, full unit suite)make update-reproducible-reqsif dependencies were changed (no dep changes)force_upload_option; no separate docs file in this repo for this command)By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.