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

Increase chamber retries to 20 #1837

Merged
merged 1 commit into from Mar 8, 2019

Conversation

3 participants
@pjdufour-truss
Copy link
Contributor

pjdufour-truss commented Mar 7, 2019

Description

This PR increase the number of retries used by chamber to 20 from 10. It looks like chamber is already using exponential backoff, so this should make acceptance tests and deploys more reliable and less likely to be stopped by rate limits for SSM.

Reviewer Notes

Is there anything you would like reviewers to give additional scrutiny?

Setup

None

Code Review Verification Steps

  • Code follows the guidelines for Logging
  • The requirements listed in
    Querying the Database Safely
    have been satisfied.
  • Any new migrations/schema changes:
    • Follow our guidelines for zero-downtime deploys (see Zero-Downtime Deploys)
    • Have been communicated to #dp3-engineering
    • Secure migrations have been tested using bin/run-prod-migrations
  • There are no aXe warnings for UI.
  • This works in IE.
  • Any new client dependencies (Google Analytics, hosted libraries, CDNs, etc) have been:
  • Tested in the Experimental environment (for changes to containers, app startup, or connection to data stores)
  • User facing changes have been reviewed by design.
  • Request review from a member of a different team.
  • Have the Pivotal acceptance criteria been met for this change?

References

Screenshots

N.A.

@pjdufour-truss pjdufour-truss force-pushed the chamber_retries branch from 5353e9d to ccbf67d Mar 7, 2019

@chrisgilmerproj
Copy link
Contributor

chrisgilmerproj left a comment

🚀 - Nice!

@codecov

This comment has been minimized.

Copy link

codecov bot commented Mar 7, 2019

Codecov Report

Merging #1837 into master will decrease coverage by 0.08%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1837      +/-   ##
==========================================
- Coverage    49.5%   49.41%   -0.09%     
==========================================
  Files         428      427       -1     
  Lines       18432    18355      -77     
  Branches     1635     1631       -4     
==========================================
- Hits         9125     9071      -54     
+ Misses       8505     8486      -19     
+ Partials      802      798       -4
@@ -76,6 +76,7 @@ commands:
- deploy:
name: Deploy app service
command: bin/do-exclusively --job-name ${CIRCLE_JOB} bin/ecs-deploy-service-container app config/app.container-definition.json ${AWS_ACCOUNT_ID}.dkr.ecr.${AWS_DEFAULT_REGION}.amazonaws.com/app:git-${CIRCLE_SHA1} $APP_ENVIRONMENT FARGATE
no_output_timeout: 20m

This comment has been minimized.

@rdhariwal

rdhariwal Mar 8, 2019

Contributor

I was curios what 'no_output_timeout' does, looks like we are overriding the default 10m timeout for the command if it doesn't spit out anything.

This is what circleci docs had to say about it:

Elapsed time the command can run without output. The string is a decimal with unit suffix, such as “20m”, “1.25h”, “5s” (default: 10 minutes)

This comment has been minimized.

@pjdufour-truss

pjdufour-truss Mar 8, 2019

Author Contributor

You got it. Some ECS commands timeout right at 10 minutes, so we want a buffer. 20 minutes seems like a good number, but 15 minutes could work too I guess.

@rdhariwal
Copy link
Contributor

rdhariwal left a comment

:shipit:

@pjdufour-truss pjdufour-truss force-pushed the chamber_retries branch from 0069409 to 215f706 Mar 8, 2019

@pjdufour-truss pjdufour-truss merged commit 0a24c3f into master Mar 8, 2019

18 of 19 checks passed

codecov/project 49.41% (-0.09%) compared to 1ea89f2
Details
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
ci/circleci: acceptance_tests_experimental Your tests passed on CircleCI!
Details
ci/circleci: acceptance_tests_local Your tests passed on CircleCI!
Details
ci/circleci: acceptance_tests_staging Your tests passed on CircleCI!
Details
ci/circleci: build_app Your tests passed on CircleCI!
Details
ci/circleci: build_migrations Your tests passed on CircleCI!
Details
ci/circleci: build_tools Your tests passed on CircleCI!
Details
ci/circleci: client_test Your tests passed on CircleCI!
Details
ci/circleci: client_test_coverage Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_mymove Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_office Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_tsp Your tests passed on CircleCI!
Details
ci/circleci: pre_deps_golang Your tests passed on CircleCI!
Details
ci/circleci: pre_deps_yarn Your tests passed on CircleCI!
Details
ci/circleci: pre_test Your tests passed on CircleCI!
Details
ci/circleci: server_test Your tests passed on CircleCI!
Details
ci/circleci: server_test_coverage Your tests passed on CircleCI!
Details
codecov/patch Coverage not affected when comparing 1ea89f2...215f706
Details

@pjdufour-truss pjdufour-truss deleted the chamber_retries branch Mar 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.