Skip to content

Commit

Permalink
Merge #595
Browse files Browse the repository at this point in the history
595: Make the number of retries configurable r=mattBrzezinski a=iamed2

This adds a `max_attempts` kwarg+field to AWSConfig, and a `max_attempts` accessor for `AbstractAWSConfig` (defaulting to 3, which is the default elsewhere in the code). 

I saw most of the tests are done with S3, and I also saw that the S3 throttling error (`SlowDown`) was not included. I added that error code and then added a test that uses it and tests throttling retries. 

---
- To see the specific tasks where the Asana app for GitHub is being used, see below:
  - https://app.asana.com/0/0/1204220867368963

Co-authored-by: Eric Davies <[email protected]>
Co-authored-by: mattBrzezinski <[email protected]>
  • Loading branch information
3 people authored Mar 20, 2023
2 parents 8251a4e + 64bf075 commit f738b28
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 5 deletions.
2 changes: 1 addition & 1 deletion Project.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ julia = "1.6"

[extras]
Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"
Suppressor = "fd094767-a336-5f1f-9728-57cf17d0bbfb"
StableRNGs = "860ef19b-820b-49d6-a774-d7a799459cd3"
Suppressor = "fd094767-a336-5f1f-9728-57cf17d0bbfb"
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
UUIDs = "cf7118a7-6976-5b1a-9a39-7adc72f591a4"

Expand Down
29 changes: 27 additions & 2 deletions src/AWSConfig.jl
Original file line number Diff line number Diff line change
@@ -1,18 +1,43 @@
abstract type AbstractAWSConfig end

# https://docs.aws.amazon.com/sdkref/latest/guide/feature-retry-behavior.html
const AWS_MAX_RETRY_ATTEMPTS = 3

"""
max_attempts(::AbstractAWSConfig) -> Int
The number of AWS request retry attempts to make.
Each backend may perform an additional layer backend-specific retries.
`AbstractAWSConfig` subtypes should allow users to override the default.
The default is 3, per the recommendations at
https://docs.aws.amazon.com/sdkref/latest/guide/feature-retry-behavior.html
"""
max_attempts(::AbstractAWSConfig) = AWS_MAX_RETRY_ATTEMPTS

mutable struct AWSConfig <: AbstractAWSConfig
credentials::Union{AWSCredentials,Nothing}
region::String
output::String
max_attempts::Int
end

# provide a default for the new field since people were using the default constructor
AWSConfig(creds, region, output) = AWSConfig(creds, region, output, AWS_MAX_RETRY_ATTEMPTS)

credentials(aws::AWSConfig) = aws.credentials
region(aws::AWSConfig) = aws.region
max_attempts(aws::AWSConfig) = aws.max_attempts

function AWSConfig(;
profile=nothing, creds=AWSCredentials(; profile=profile), region=nothing, output="json"
profile=nothing,
creds=AWSCredentials(; profile=profile),
region=nothing,
output="json",
max_attempts=AWS_MAX_RETRY_ATTEMPTS,
)
region = @something region aws_get_region(; profile=profile)
return AWSConfig(creds, region, output)
return AWSConfig(creds, region, output, max_attempts)
end

"""
Expand Down
3 changes: 2 additions & 1 deletion src/utilities/request.jl
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ function submit_request(aws::AbstractAWSConfig, request::Request; return_headers
"LimitExceededException",
"RequestThrottled",
"PriorRequestNotComplete",
"SlowDown",
]

request.headers["User-Agent"] = user_agent[]
Expand Down Expand Up @@ -190,7 +191,7 @@ function submit_request(aws::AbstractAWSConfig, request::Request; return_headers
return false
end

delays = AWSExponentialBackoff(; max_attempts=3)
delays = AWSExponentialBackoff(; max_attempts=max_attempts(aws))

retry(upgrade_error(get_response); check=check, delays=delays)()

Expand Down
2 changes: 1 addition & 1 deletion src/utilities/utilities.jl
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ end
# https://docs.aws.amazon.com/sdkref/latest/guide/feature-retry-behavior.html
# Default values for AWS's `standard` retry mode. Note: these can be overridden elsewhere.
Base.@kwdef struct AWSExponentialBackoff
max_attempts::Int = 3
max_attempts::Int = AWS_MAX_RETRY_ATTEMPTS
max_backoff::Float64 = 20.0
rng::AbstractRNG = Random.GLOBAL_RNG
end
Expand Down
59 changes: 59 additions & 0 deletions test/AWS.jl
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,65 @@ end
@test String(response.body) == Patches.body
end

@testset "Default throttling" begin
request = Request(;
service="s3",
api_version="api_version",
request_method="GET",
url="https://s3.us-east-1.amazonaws.com/sample-bucket",
use_response_type=true,
)

retries = Ref{Int}(0)
exception = apply(Patches._throttling_patch(retries)) do
try
AWS.submit_request(aws, request)
return nothing
catch e
if e isa AWSException
return e
else
rethrow()
end
end
end

@test exception isa AWSException
@test exception.code == "SlowDown"
@test retries[] == AWS.max_attempts(aws)
end

@testset "Custom throttling" begin
aws = AWS.AWSConfig(; max_attempts=1)
@test AWS.max_attempts(aws) == 1

request = Request(;
service="s3",
api_version="api_version",
request_method="GET",
url="https://s3.us-east-1.amazonaws.com/sample-bucket",
use_response_type=true,
)

retries = Ref{Int}(0)
exception = apply(Patches._throttling_patch(retries)) do
try
AWS.submit_request(aws, request)
return nothing
catch e
if e isa AWSException
return e
else
rethrow()
end
end
end

@test exception isa AWSException
@test exception.code == "SlowDown"
@test retries[] == AWS.max_attempts(aws)
end

@testset "Not authorized" begin
request = Request(;
service="s3",
Expand Down
21 changes: 21 additions & 0 deletions test/patch.jl
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,27 @@ function _aws_http_request_patch(response::AWS.Response=_response())
return p
end

function _throttling_patch(retries::Ref{Int})
status = 503
body = """
<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n
<Error>
<Code>SlowDown</Code>
<Message>Please reduce your request rate</Message>
<Resource>/mybucket/myfoto.jpg</Resource>
<RequestId>4442587FB7D0A2F9</RequestId>
</Error>
"""

p = @patch function AWS._http_request(::AWS.AbstractBackend, request::Request, ::IO)
retries[] += 1
resp = _response(; status=status, body=body).response
err = HTTP.StatusError(status, request.request_method, request.resource, resp)
throw(AWS.AWSException(err, body))
end
return p
end

_cred_file_patch = @patch function dot_aws_credentials_file()
return ""
end
Expand Down

0 comments on commit f738b28

Please sign in to comment.