feat: prevent deploy throttle with customizable interval for DescribeStack via env by BourgoisMickael · Pull Request #3500 · aws/aws-sam-cli

BourgoisMickael · 2021-11-28T00:53:29Z

Which issue(s) does this change fix?

Related PR: #2748

Why is this change necessary?

sam deploy often gets throttled because it sends 2 DescribeStack per seconds.
As commented in #2748 (comment) this is problematic with a lot of pipeline in parallel that deploy stacks during 10 minutes.
Pipelines don't need real time updates so we can't to customize the interval between DescribeStack

How does it address the issue?

Customize the interval between DescribeStack via environment variable SAM_CLI_DEPLOY_DESCRIBE_STACK_INTERVAL

What side effects does this change have?

None

Checklist

Add input/output type hints to new functions/methods
Write design document (Do I need to write a design document?)
Write unit tests
Write/update functional tests
Write/update integration tests
make pr passes
make update-reproducible-reqs if dependencies were changed
Write documentation

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

samcli/lib/deploy/deployer.py

jfuss · 2022-03-02T19:56:27Z

@BourgoisMickael Sorry for the mix messaging you are getting from the team. I am trying to clarify with the team now. Personally, I don't think it matters a whole lot and we just need to align on one or the other to start. If we pick one and want the other later, Click has a way to default values from an Env Var so we shouldn't be stuck regardless.

BourgoisMickael · 2022-03-12T23:32:30Z

Ok, let me know when there are news about this.

I can also update the documentation later to describe this environment variable.

jfuss · 2022-03-21T17:16:45Z

@BourgoisMickael Chatting with the team, we can accept the env var. If we want to move towards a cli option, we still can later on through click.

The changes you have proposed seem reasonable but I would like if you could move the env var into the command and pass this through to the deployer. This keeps the configuration on the "cli" level while making the deployer more like a lib than reading the env directly.

The thing I am thinking through is should we make the env var more generic? SAM_CLI_DEPLOY_DESCRIBE_STACK_INTERVAL is fine but I maybe it is worth doing something like SAM_CLI_THROTTLE_RATE (naming is hard). If we can find a good general name, then we could re-use this configuration in other places. Maybe it's not worth it, but would like to here your thoughts.

BourgoisMickael · 2022-04-04T02:04:32Z

@jfuss I renamed the variable to SAM_CLI_POLL_DELAY. I think it describes best what it does and it could be reused for other poller like for the sam delete

samcli/commands/deploy/deploy_context.py

moelasmar · 2022-04-21T19:48:08Z

Thanks @BourgoisMickael for your contribution. This PR looks good to me, just a small change requested.

Otherwise if there is an exceptions, every next iteration won't sleep and it will spam the server

samcli/lib/deploy/deployer.py

jfuss

LGTM. Thanks for taking the time to work on this.

moelasmar

thanks @BourgoisMickael for your contribution

Dismissing old reviews

jfuss · 2022-05-03T21:24:15Z

@BourgoisMickael Thanks for pushing this through. There is no further action on you but I need to figure out how/what docs need to get updated. I will merge once I kick off that process so we don't accidentally release without the doc updates.

jfuss · 2022-05-17T15:56:08Z

I was thinking about this more. I think added something to the help text (which will get added to our docs) is a good first step. Maybe not enough but I think this is enough to get this merged and handle updates as follow ups. I will push a commit when a I get some time and get this PR merged.

smurphnturf · 2022-05-26T01:42:24Z

Love this thank you!! :)

BourgoisMickael · 2022-05-26T19:21:36Z

I just found a bug this feature introduces #3910 @smurphnturf

* feat: updating app templates repo hash with (032e70a66732ed18db7ba6c8b13293cada3de740) (#3849) Co-authored-by: GitHub Action <action@github.com> * chore: Added Support for Test Account Management (#3845) * Added get_testing_resources.py * Added Setting Test Env Vars * Added send-task-success to Windows * Updated MANAGED_TEST_RESOURCE_STACK_NAME * Fixed Region and Bash Env Var * Added venv to get_testing_resources * Fixed CredentialDistributionLambda region * Added Cred Protection for Testing * Fixed Python Version * Added venv for pythonn3.9 * Fixed FunctionError Check * Fixed Cred Key Names * Fixed jq echo Syntax * Removed test_env_var Debug Echo * Added Docker Authentication and Cred Swap * Added Debug STS Caller Identity * Removed Debug STS * Updated Lambda Max Attempts to Avoid Calling Multiple Times * Fixed Ubuntu NoAvailableAccountException Check * Added Exit to NoAvailableAccountException * Updated to Exit Code Check * Fixed Ubuntu jq and Windows Newlines * Fixed Windows on_finish Newlines * Removed Docker for Non Canary CI * Updated to Echo Static Error Message * Added Docs for get_testing_resources * Fixed Ubuntu Send TaskToken Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> Co-authored-by: Qingchuan Ma <69653965+qingchm@users.noreply.github.com> * fix: command should not fail when global config fails to write (#3859) * fix: don't fail when global config fails to write * fix tests & change to warning message for failing to read or write * Bump Lambda Builder version to 1.16.0 (#3865) * feat: updating app templates repo hash with (22bb8ca9d29bea25f535c29a919fa4e963a2755b) (#3867) Co-authored-by: GitHub Action <action@github.com> Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com> * Nodejs16 support for samcli (#419) (#3866) * Dotnet6 support for samcli * add nodejs16.x to test_workflow and architecture.py * Fix the test tests/unit/commands/init/test_cli.py::TestCli::test_init_cli_node * Fix test_init_cli_node by using a new dict instead of extra_context_as_json Co-authored-by: Aboelhamd Aly <aboelha@amazon.com> * chore: Version bump to 1.49.0 (#3868) * chore: Version bump to 1.49.0 * Fix init test for node * Fix init test for node * Black reformat * fix: iac canaries (#3874) * fix: iac canaries credentials * add cleanup step as well * install p39 venv * install jq * fix: Lock functions from building with same codeuri at the same time (#3862) * Lock functions from building with same codeuri at the same time * Remove unused import * Remove bin. Add check for befor entering lock context * Remove unneeded files, updated integration tests Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> * feat: updating app templates repo hash with (70afc6988b77387818e3be7f0d2a836878099fb5) (#3869) Co-authored-by: GitHub Action <action@github.com> Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> * fix: Use abs path to upload stackset template on S3 (#3818) * fix: Use abs path to upload stackset template on S3 * test: ✅ Add missing integration test for stack sets package Co-authored-by: Massimo Maino <massimo.maino@claudacademy.com> Co-authored-by: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com> Co-authored-by: mingkun2020 <68391979+mingkun2020@users.noreply.github.com> Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> * fix: Add in trace event revision to prevent partial trace printing (#3876) * Adding revision in traces to resolve partial traces * Correcting file name * Black reformatting Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> * chore: bump version to 1.50.0 (#3878) * feat: sam init support for x-ray tracing (API Gateway) (#3856) * Set the global parameter Api.TracingEnabled to True to enable API Gateway X-Ray tracing when the Trace option is specified. * passed unittest/make pr. * reset make black * create separate methods * use generic function Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> * fix: multivalue query params http (#3788) * fixes multivalue query param requests for HTTP APIs When a query parameter is specified multiple times in a URL, e.g. `path?mykey=1&mykey=2&mykey=3` this should create an event with all appropriate values. Prior to this commit, when using `sam local start-api` with an HTTP API, only the first value is present in the queryStringParameters of the event. In payload format v2 [1] used in HTTP APIs, multiple values should be combined with commas and included in the queryStringParameters field. This commit fixes the behavior for query string parameters. [1] https://docs.aws.amazon.com/apigateway/latest/developerguide/http-api-develop-integrations-lambda.html * black reformat * apply suggestion * add test Co-authored-by: Jason LaPier <jason@heyfieldday.com> Co-authored-by: Qingchuan Ma <69653965+qingchm@users.noreply.github.com> Co-authored-by: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com> Co-authored-by: Jacob Fuss <32497805+jfuss@users.noreply.github.com> Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> * feat: Nested stack support for sync watch (#3853) * Nested stack support for sync watch * Addressing comments * Black reformatting * Refactoring validator * Change trigger creation failure from debug to warning * Improve logging * Changing validation behaviour to not fail initialization * Refactoring * Refactoring method names and updating docstring Co-authored-by: mingkun2020 <68391979+mingkun2020@users.noreply.github.com> * feat: sam traces integration test (#3618) add traces integration test Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> Co-authored-by: Qingchuan Ma <69653965+qingchm@users.noreply.github.com> * chore: fixing the trace event class to support trace_id option (#3879) * feat: updating app templates repo hash with (9f2cb3069f0ac3a21c2dec58adfb9450d26086e2) (#3880) Co-authored-by: GitHub Action <action@github.com> * feat: improve new PR and issue labeling for triaging (#3890) * feat: improve new PR and issue labeling for triaging * add 'anything else' template and disable blank templates, in order to ensure they are all auto-labeled for triaging * anything else -> other * feat: nested stack support for sam logs (#3881) * feat: nested stack support for sam logs * update command documentation * update to use key value pairs instead of list of resources * update expected values for infra sync integ tests * feat: updating app templates repo hash with (6d61039f51b2d12e2f9e339eba0c023ddbf395ae) (#3892) Co-authored-by: GitHub Action <action@github.com> * feat: prevent deploy throttle with customizable interval for DescribeStack via env (#3500) * Customize deploy describe stack sleep interval via environment * Test deployer with custom describe stack interval * Move and rename env var for deploy polling * Fix tests on deploy poller * Move env var from context to command * Fix tests for command * Reset retry_attemps on success after exception Otherwise if there is an exceptions, every next iteration won't sleep and it will spam the server * Add details on what to set in the deploy help text output. Co-authored-by: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com> Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> Co-authored-by: Jacob Fuss <32497805+jfuss@users.noreply.github.com> Co-authored-by: Jacob Fuss <jfuss@users.noreply.github.com> * feat: make java build scope changes default (#3896) * feat: make java build scope changes default * update lambda builders version * Add explicit --no-cached option (#3889) * added --no-cached option to override cached = true in samconfig.toml * Added test to check that the no-cached option overrides the cached = true setting in samconfig.toml * Added integration tests to verify the --no-cached option * Reformatted file * Modified integration test and testdata path source Co-authored-by: Andrew Zhan <zhandr@amazon.com> * feat: ADL nested stack support (#3893) * feat: add nested stack support for auto dependency layer * add/update unit tests * address comments * use different variable name for sanitized function logical id Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com> Co-authored-by: mingkun2020 <68391979+mingkun2020@users.noreply.github.com> * test: Integration Tests for Sync Watch with Nested Stacks (#3909) * Generalize setup of existing watch tests * Add tests for infra nested stacks * Add tests for code sync watch * Paramaterize ADL config * Black reformat Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> * fix: Add lockchain and status check to prevent sync flows race condition on function update (#3905) * Initial investigation to fix * Add lockchain and status check to prevent function config update collision with function update * Adding in unit tests * Black reformatting * Improving unit test * Add integration test * Refactoring to address comments * Ignore type check for boto client call * Refactoring * Added lockchain and wait mechanism for image functions * Improving docstring * Sync code nested (#3887) add nested stack support for sam sync code * fix: correct node16 debugging configuration (#3913) Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> * feat: new release of RIE: 1.6 (#3897) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: GitHub Action <action@github.com> Co-authored-by: Wilton_ <CoshUS@users.noreply.github.com> Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> Co-authored-by: Qingchuan Ma <69653965+qingchm@users.noreply.github.com> Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com> Co-authored-by: Aboelhamd Aly <aboelha@amazon.com> Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com> Co-authored-by: Massimo Maino <maintux@gmail.com> Co-authored-by: Massimo Maino <massimo.maino@claudacademy.com> Co-authored-by: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com> Co-authored-by: mingkun2020 <68391979+mingkun2020@users.noreply.github.com> Co-authored-by: iwasa <Tak1wa@users.noreply.github.com> Co-authored-by: Jason LaPier <jason@heyfieldday.com> Co-authored-by: Jacob Fuss <32497805+jfuss@users.noreply.github.com> Co-authored-by: Ruperto Torres <86501267+torresxb1@users.noreply.github.com> Co-authored-by: Mickael Bourgois <35171168+BourgoisMickael@users.noreply.github.com> Co-authored-by: Jacob Fuss <jfuss@users.noreply.github.com> Co-authored-by: Andrew Zhan <zhandr@amazon.com> Co-authored-by: JadenSimon <31319484+JadenSimon@users.noreply.github.com> Co-authored-by: Renato Valenzuela <37676028+valerena@users.noreply.github.com>

…Stack via env (aws#3500) * Customize deploy describe stack sleep interval via environment * Test deployer with custom describe stack interval * Move and rename env var for deploy polling * Fix tests on deploy poller * Move env var from context to command * Fix tests for command * Reset retry_attemps on success after exception Otherwise if there is an exceptions, every next iteration won't sleep and it will spam the server * Add details on what to set in the deploy help text output. Co-authored-by: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com> Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> Co-authored-by: Jacob Fuss <32497805+jfuss@users.noreply.github.com> Co-authored-by: Jacob Fuss <jfuss@users.noreply.github.com>

github-actions bot added the pr/external label Nov 28, 2021

jbernales5 approved these changes Dec 2, 2021

View reviewed changes

jbernales5 mentioned this pull request Dec 2, 2021

feat: add --initial-reporting-sleep for sam deploy to fix (Throttling) when calling the DescribeStacks #2748

Closed

8 tasks

aws-sam-cli-bot added the maintainer/need-response label Dec 5, 2021

c2tarun previously requested changes Jan 6, 2022

View reviewed changes

samcli/lib/deploy/deployer.py Outdated Show resolved Hide resolved

c2tarun removed the maintainer/need-response label Jan 6, 2022

BourgoisMickael requested a review from c2tarun January 13, 2022 17:25

aws-sam-cli-bot added the maintainer/need-followup label Jan 16, 2022

aws-sam-cli-bot removed the maintainer/need-followup label Jan 26, 2022

aws-sam-cli-bot added the maintainer/need-followup label Mar 15, 2022

jfuss self-assigned this Mar 21, 2022

aws-sam-cli-bot removed the maintainer/need-followup label Mar 22, 2022

BourgoisMickael added 4 commits April 4, 2022 02:43

Customize deploy describe stack sleep interval via environment

bbf2889

Test deployer with custom describe stack interval

132b8b9

Move and rename env var for deploy polling

4a54304

Fix tests on deploy poller

dee5ac3

aws-sam-cli-bot added the maintainer/need-followup label Apr 6, 2022

Merge branch 'develop' into fix/throttle

ed8bac0

moelasmar requested changes Apr 21, 2022

View reviewed changes

samcli/commands/deploy/deploy_context.py Outdated Show resolved Hide resolved

aws-sam-cli-bot removed the maintainer/need-followup label Apr 22, 2022

BourgoisMickael added 3 commits April 24, 2022 04:32

Move env var from context to command

22ef188

Fix tests for command

41ba14b

Reset retry_attemps on success after exception

5ab82fd

Otherwise if there is an exceptions, every next iteration won't sleep and it will spam the server

BourgoisMickael commented Apr 24, 2022

View reviewed changes

samcli/lib/deploy/deployer.py Show resolved Hide resolved

aws-sam-cli-bot added the maintainer/need-followup label Apr 27, 2022

aws-sam-cli-bot added maintainer/need-followup and removed maintainer/need-followup labels Apr 28, 2022

jfuss approved these changes May 3, 2022

View reviewed changes

moelasmar approved these changes May 3, 2022

View reviewed changes

Merge branch 'develop' into fix/throttle

16149d9

aws-sam-cli-bot removed the maintainer/need-followup label May 4, 2022

Merge branch 'develop' into fix/throttle

5ede153

jfuss and others added 4 commits May 17, 2022 10:56

Merge branch 'develop' into fix/throttle

a44db8e

Add details on what to set in the deploy help text output.

6d8d967

Merge branch 'develop' into fix/throttle

9e84c94

Merge branch 'develop' into fix/throttle

84a702e

mndeveci enabled auto-merge (squash) May 25, 2022 19:54

mndeveci merged commit 9f33db9 into aws:develop May 25, 2022

This was referenced May 26, 2022

Bug: deploy with large SAM_CLI_POLL_DELAY has a wrong display (nightly) #3910

Closed

Feature request: remove unnecessary describe-stack calls during deploy #3911

Closed

BourgoisMickael mentioned this pull request May 30, 2022

Fix deploy with long poll delay events order and stop condition #3919

Merged

3 tasks

tim-finnigan mentioned this pull request Apr 4, 2023

Don't call describe_stacks in aws cloudformation deploy aws/aws-cli#3186

Closed

alex-gausman mentioned this pull request Jul 31, 2023

Feature Addition: User Configurable Stack Polling Interval aws/aws-extensions-for-dotnet-cli#287

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: prevent deploy throttle with customizable interval for DescribeStack via env#3500

feat: prevent deploy throttle with customizable interval for DescribeStack via env#3500
mndeveci merged 14 commits intoaws:developfrom
BourgoisMickael:fix/throttle

BourgoisMickael commented Nov 28, 2021

Uh oh!

Uh oh!

jfuss commented Mar 2, 2022

Uh oh!

BourgoisMickael commented Mar 12, 2022

Uh oh!

jfuss commented Mar 21, 2022

Uh oh!

BourgoisMickael commented Apr 4, 2022 •

edited

Loading

Uh oh!

Uh oh!

moelasmar commented Apr 21, 2022

Uh oh!

Uh oh!

jfuss left a comment

Uh oh!

moelasmar left a comment

Uh oh!

jfuss commented May 3, 2022

Uh oh!

jfuss commented May 17, 2022

Uh oh!

smurphnturf commented May 26, 2022

Uh oh!

BourgoisMickael commented May 26, 2022 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

Conversation

BourgoisMickael commented Nov 28, 2021

Which issue(s) does this change fix?

Why is this change necessary?

How does it address the issue?

What side effects does this change have?

Checklist

Uh oh!

Uh oh!

jfuss commented Mar 2, 2022

Uh oh!

BourgoisMickael commented Mar 12, 2022

Uh oh!

jfuss commented Mar 21, 2022

Uh oh!

BourgoisMickael commented Apr 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

moelasmar commented Apr 21, 2022

Uh oh!

Uh oh!

jfuss left a comment

Choose a reason for hiding this comment

Uh oh!

moelasmar left a comment

Choose a reason for hiding this comment

Uh oh!

jfuss commented May 3, 2022

Uh oh!

jfuss commented May 17, 2022

Uh oh!

smurphnturf commented May 26, 2022

Uh oh!

BourgoisMickael commented May 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

BourgoisMickael commented Apr 4, 2022 •

edited

Loading

BourgoisMickael commented May 26, 2022 •

edited

Loading