-
Notifications
You must be signed in to change notification settings - Fork 246
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(pacmak): retry select external command invocations #2013
Conversation
When generating packages, `jsii-pacmak` will defer to external tools for compilation (`dotnet`, `mvn`, `pip`, ...). Those commands will typically require some kind of network transfer (e.g: to resolve dependencies from their standard package registry), which is susceptible to fail (due to poor network conditions, getting throttled in CI/CD, etc...). In order to reduce the likelihood that such failures will interrupt the user workflow abruptly (forcing a manual retry, and possibly breaking CI/CD workflows), back-off and retries will be performed when these commands fail (up to 5 tentatives in total, with exponential back-off using a 150ms base cooldown and a factor of 2).
88cd919
to
67b4736
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did the retry mechanism need to be rewritten instead of simply turned on?
packages/jsii-pacmak/lib/util.ts
Outdated
* | ||
* @default 5 | ||
*/ | ||
maxTries?: number; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maxAttempts
then? This word grates on me for some reason.
For some reason I only noticed there was already support for retries after writing the new retry wrapper. And the old retry mechanism lacked support for back-off, making it questionnable for working around throttles / DDOS protections. |
Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it! |
Merging (with squash)... |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Merging (with squash)... |
When generating packages,
jsii-pacmak
will defer to external tools forcompilation (
dotnet
,mvn
,pip
, ...). Those commands will typicallyrequire some kind of network transfer (e.g: to resolve dependencies from
their standard package registry), which is susceptible to fail (due to
poor network conditions, getting throttled in CI/CD, etc...).
In order to reduce the likelihood that such failures will interrupt the
user workflow abruptly (forcing a manual retry, and possibly breaking
CI/CD workflows), back-off and retries will be performed when these
commands fail (up to 5 tentatives in total, with exponential back-off
using a 150ms base cooldown and a factor of 2).
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.