Re-merge "Update aws-sdk gem version" #14817
Conversation
These aren't listed as required on http://docs.aws.amazon.com/sdkforruby/api/Aws/CloudFront/Types/DistributionConfig.html however API calls are failing without them. Both are set to the default value.
joshlory
changed the title
lib/cdo/aws/cloudfront.rb
Outdated
| custom_origin_config: { | ||
| origin_read_timeout: 5, | ||
| origin_keepalive_timeout: 30 | ||
| }, |
There was a problem hiding this comment.
these lines need to be removed - only one of s3_origin_config or custom_origin_config are required.
There was a problem hiding this comment.
Do I add these options to s3_origin_config or remove them entirely?
|
Updated! Is there any way to test these changes before the merge to staging? |
There was a problem hiding this comment.
LGTM. For testing, test_cloudfront.rb ensures that the client is passing valid structures to the AWS API, but the AWS API itself performs extra validation that we can't test without a complete integration test. I've actually written such a test (see cookbooks/cdo-varnish/test/cookbooks/varnish_test), but since updating a CloudFront distribution can take up to an hour, it's currently just a manual test, so not feasible to add it to our continuous integration. So just merging to staging is fine
|
@joshlory, our pipeline is pretty clean right now (about to get my third green DTT), if you want to merge this, I'm happy to deal with any fallout as DOTD. |
|
The https://codedotorg.slack.com/archives/C03CK8E51/p1494301821954043 |
| } | ||
| }, | ||
| origin_read_timeout: 5, | ||
| origin_keepalive_timeout: 30 |
There was a problem hiding this comment.
@joshlory turns out these values are mismatched, they should have been origin_read_timeout: 30, origin_keepalive_timeout: 5 to match the defaults. sorry for not picking this up earlier
|
Also needed one more config option: eeaa8af. |
Re-merge PR #14798.