From 18b93ca5265cc3910a7a10ff1669095e09ab2976 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Sat, 10 Jul 2021 02:00:34 +0200 Subject: [PATCH 01/11] provide hooks for custom backends --- src/utilities/request.jl | 25 ++++++++++++++++++++++--- src/utilities/utilities.jl | 1 + test/patch.jl | 4 ++-- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/utilities/request.jl b/src/utilities/request.jl index 1d3b56323d..23db31f6ab 100644 --- a/src/utilities/request.jl +++ b/src/utilities/request.jl @@ -1,3 +1,19 @@ +# this is used to allow custom dispatches to `_http_request` +abstract type AbstractBackend end + +""" + AWS.HTTPBackend <: AWS.AbstractBackend + +An `HTTPBackend` can hold default `http_options::AbstractDict{Symbol,<:Any}` +to pass to HTTP.jl, which can be overwritten per-request by any `http_options` +supplied there. +""" +Base.@kwdef struct HTTPBackend <: AbstractBackend + http_options::AbstractDict{Symbol,<:Any}=LittleDict{Symbol,String}() +end + +const DEFAULT_BACKEND = Ref{Union{Nothing, AbstractBackend}}(HTTPBackend()) + Base.@kwdef mutable struct Request service::String api_version::String @@ -13,6 +29,7 @@ Base.@kwdef mutable struct Request http_options::AbstractDict{Symbol,<:Any}=LittleDict{Symbol,String}() return_raw::Bool=false response_dict_type::Type{<:AbstractDict}=LittleDict + backend::AbstractBackend=DEFAULT_BACKEND[] end @@ -54,7 +71,7 @@ function submit_request(aws::AbstractAWSConfig, request::Request; return_headers @repeat 3 try credentials(aws) === nothing || sign!(aws, request) - response = @mock _http_request(request) + response = @mock _http_request(request.backend, request) if response.status in REDIRECT_ERROR_CODES if HTTP.header(response, "Location") != "" @@ -141,7 +158,9 @@ function submit_request(aws::AbstractAWSConfig, request::Request; return_headers end -function _http_request(request::Request) +function _http_request(http_backend::HTTPBackend, request::Request) + http_options = merge(http_backend.http_options, request.http_options) + @repeat 4 try http_stack = HTTP.stack(redirect=false, retry=false, aws_authorization=false) @@ -157,7 +176,7 @@ function _http_request(request::Request) request.content; require_ssl_verification=false, response_stream=request.response_stream, - request.http_options... + http_options... ) catch e # Base.IOError is needed because HTTP.jl can often have errors that aren't diff --git a/src/utilities/utilities.jl b/src/utilities/utilities.jl index 9df8b18e9b..f10f561236 100644 --- a/src/utilities/utilities.jl +++ b/src/utilities/utilities.jl @@ -68,6 +68,7 @@ function _extract_common_kw_args(service, args) headers=LittleDict{String, String}(_pop!(args, "headers", [])), http_options=_pop!(args, "http_options", LittleDict{Symbol, String}()), response_dict_type=_pop!(args, "response_dict_type", LittleDict), + backend=_pop!(args, "backend", DEFAULT_BACKEND[]), ) end diff --git a/test/patch.jl b/test/patch.jl index 1109544d80..e0edb195ec 100644 --- a/test/patch.jl +++ b/test/patch.jl @@ -56,7 +56,7 @@ end function _aws_http_request_patch(response::HTTP.Messages.Response=_response()) - return @patch AWS._http_request(request::Request) = response + return @patch AWS._http_request(::Any, request::Request) = response end _cred_file_patch = @patch function dot_aws_credentials_file() @@ -73,7 +73,7 @@ _web_identity_patch = function (; session_token="web_session_token", role_arn="arn:aws:sts:::assumed-role/role-name", ) - @patch function AWS._http_request(request) + @patch function AWS._http_request(::AWS.HTTPBackend, request) params = Dict(split.(split(request.content, '&'), '=')) creds = Dict( "AccessKeyId" => access_key, From c008bf4e0f3c87e28e3c971c814acfe678fc4e7e Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Tue, 13 Jul 2021 20:45:04 +0200 Subject: [PATCH 02/11] `Base.@kwdef` does not like subtyping on Julia-1.0 --- src/utilities/request.jl | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/utilities/request.jl b/src/utilities/request.jl index 23db31f6ab..78d58f76f2 100644 --- a/src/utilities/request.jl +++ b/src/utilities/request.jl @@ -8,10 +8,12 @@ An `HTTPBackend` can hold default `http_options::AbstractDict{Symbol,<:Any}` to pass to HTTP.jl, which can be overwritten per-request by any `http_options` supplied there. """ -Base.@kwdef struct HTTPBackend <: AbstractBackend - http_options::AbstractDict{Symbol,<:Any}=LittleDict{Symbol,String}() +struct HTTPBackend{T<:AbstractDict{Symbol,<:Any}} <: AbstractBackend + http_options::T end +HTTPBackend() = HTTPBackend(LittleDict{Symbol,String}()) + const DEFAULT_BACKEND = Ref{Union{Nothing, AbstractBackend}}(HTTPBackend()) Base.@kwdef mutable struct Request From d54a684457c0e6a576d15a5adefdf3b6ab8d04aa Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Tue, 13 Jul 2021 21:28:53 +0200 Subject: [PATCH 03/11] Update src/utilities/request.jl --- src/utilities/request.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utilities/request.jl b/src/utilities/request.jl index 78d58f76f2..dca9a21439 100644 --- a/src/utilities/request.jl +++ b/src/utilities/request.jl @@ -8,8 +8,8 @@ An `HTTPBackend` can hold default `http_options::AbstractDict{Symbol,<:Any}` to pass to HTTP.jl, which can be overwritten per-request by any `http_options` supplied there. """ -struct HTTPBackend{T<:AbstractDict{Symbol,<:Any}} <: AbstractBackend - http_options::T +struct HTTPBackend <: AbstractBackend + http_options::AbstractDict{Symbol,<:Any} end HTTPBackend() = HTTPBackend(LittleDict{Symbol,String}()) From 6ce13d71682cfbcb9a5305226cc536c54622e0a2 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Wed, 14 Jul 2021 14:51:29 +0200 Subject: [PATCH 04/11] Update test/patch.jl Co-authored-by: Curtis Vogt --- test/patch.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/patch.jl b/test/patch.jl index e0edb195ec..b1080a184c 100644 --- a/test/patch.jl +++ b/test/patch.jl @@ -56,7 +56,7 @@ end function _aws_http_request_patch(response::HTTP.Messages.Response=_response()) - return @patch AWS._http_request(::Any, request::Request) = response + return @patch AWS._http_request(::AWS.HTTPBackend, request::Request) = response end _cred_file_patch = @patch function dot_aws_credentials_file() From b0db6e3cfbd53353e9ded56fa910b52b0699faeb Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Wed, 14 Jul 2021 14:51:39 +0200 Subject: [PATCH 05/11] `DEFAULT_BACKEND` cannot be `nothing` --- src/utilities/request.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utilities/request.jl b/src/utilities/request.jl index dca9a21439..0f2b48eeda 100644 --- a/src/utilities/request.jl +++ b/src/utilities/request.jl @@ -14,7 +14,7 @@ end HTTPBackend() = HTTPBackend(LittleDict{Symbol,String}()) -const DEFAULT_BACKEND = Ref{Union{Nothing, AbstractBackend}}(HTTPBackend()) +const DEFAULT_BACKEND = Ref{AbstractBackend}(HTTPBackend()) Base.@kwdef mutable struct Request service::String From ed6b88a9bf9adb7bd1b5812b175d8cadcb3b4d20 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Fri, 16 Jul 2021 16:19:12 +0200 Subject: [PATCH 06/11] add tests for `HTTPBackend` --- src/utilities/request.jl | 4 ++-- test/AWS.jl | 44 ++++++++++++++++++++++++++++++++++++++++ test/patch.jl | 11 ++++++++++ 3 files changed, 57 insertions(+), 2 deletions(-) diff --git a/src/utilities/request.jl b/src/utilities/request.jl index 0f2b48eeda..c59a754243 100644 --- a/src/utilities/request.jl +++ b/src/utilities/request.jl @@ -12,7 +12,7 @@ struct HTTPBackend <: AbstractBackend http_options::AbstractDict{Symbol,<:Any} end -HTTPBackend() = HTTPBackend(LittleDict{Symbol,String}()) +HTTPBackend(; kwargs...) = HTTPBackend(LittleDict(kwargs)) const DEFAULT_BACKEND = Ref{AbstractBackend}(HTTPBackend()) @@ -170,7 +170,7 @@ function _http_request(http_backend::HTTPBackend, request::Request) request.response_stream = Base.BufferStream() end - return HTTP.request( + return @mock HTTP.request( http_stack, request.request_method, HTTP.URI(request.url), diff --git a/test/AWS.jl b/test/AWS.jl index 29a9e80bed..7d95b5b151 100644 --- a/test/AWS.jl +++ b/test/AWS.jl @@ -351,6 +351,50 @@ end end end +@testset "HTTPBackend" begin + request = Request( + service="s3", + api_version="api_version", + request_method="GET", + url="https://s3.us-east-1.amazonaws.com/sample-bucket", + ) + apply(Patches._http_options_patch) do + # No default options + @test isempty(AWS._http_request(request.backend, request)) + + # We can pass options via the backend + custom_backend = AWS.HTTPBackend(Dict(:connection_limit => 5)) + @test custom_backend isa AWS.AbstractBackend + @test AWS._http_request(custom_backend, request) == Dict(:connection_limit => 5) + + # We can pass options per-request + request.http_options = Dict(:pipeline_limit => 20) + @test AWS._http_request(request.backend, request) == Dict(:pipeline_limit => 20) + @test AWS._http_request(custom_backend, request) == Dict(:pipeline_limit => 20, :connection_limit => 5) + + # per-request options override backend options: + custom_backend = AWS.HTTPBackend(Dict(:pipeline_limit => 5)) + @test AWS._http_request(custom_backend, request) == Dict(:pipeline_limit => 20) + end + + # Let's test setting the default backend + try + AWS.DEFAULT_BACKEND[] = AWS.HTTPBackend(; connection_limit = 3) + request = Request( + service="s3", + api_version="api_version", + request_method="GET", + url="https://s3.us-east-1.amazonaws.com/sample-bucket", + ) + @test request.backend.http_options == Dict(:connection_limit => 3) + apply(Patches._http_options_patch) do + @test AWS._http_request(request.backend, request) == Dict(:connection_limit => 3) + end + finally + AWS.DEFAULT_BACKEND[] = AWS.HTTPBackend() + end +end + @testset "_generate_rest_resource" begin request_uri = "/{Bucket}/{Key+}" args = Dict{String, Any}( diff --git a/test/patch.jl b/test/patch.jl index b1080a184c..cb9156105c 100644 --- a/test/patch.jl +++ b/test/patch.jl @@ -107,4 +107,15 @@ _instance_metadata_timeout_patch = @patch function HTTP.request(method::String, throw(HTTP.ConnectionPool.ConnectTimeout("169.254.169.254", "80")) end +# This patch causes `HTTP.request` to return all of its keyword arguments +# except `require_ssl_verification` and `response_stream`. This is used to +# test which other options are being passed to `HTTP.Request` inside of +# `_http_request`. +_http_options_patch = @patch function HTTP.request(args...; kwargs...) + options = Dict(kwargs) + delete!(options, :require_ssl_verification) + delete!(options, :response_stream) + return options +end + end From 8a419aa797ea8735c3f4d94ad7c9407a8cbb5dd4 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Fri, 16 Jul 2021 17:00:23 +0200 Subject: [PATCH 07/11] workaround issue --- src/utilities/request.jl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/utilities/request.jl b/src/utilities/request.jl index c59a754243..a4d23e0449 100644 --- a/src/utilities/request.jl +++ b/src/utilities/request.jl @@ -12,7 +12,9 @@ struct HTTPBackend <: AbstractBackend http_options::AbstractDict{Symbol,<:Any} end -HTTPBackend(; kwargs...) = HTTPBackend(LittleDict(kwargs)) +function HTTPBackend(; kwargs...) + isempty(kwargs) ? HTTPBackend(LittleDict{Symbol, Any}()) : HTTPBackend(LittleDict(kwargs)) +end const DEFAULT_BACKEND = Ref{AbstractBackend}(HTTPBackend()) From 2104be69f3710e0ca3b13b152e56c6a6f93ca57e Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Mon, 19 Jul 2021 11:47:55 +0200 Subject: [PATCH 08/11] Apply suggestions from code review Co-authored-by: Curtis Vogt --- src/utilities/request.jl | 2 +- test/patch.jl | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/utilities/request.jl b/src/utilities/request.jl index a4d23e0449..ec8270afc1 100644 --- a/src/utilities/request.jl +++ b/src/utilities/request.jl @@ -1,4 +1,4 @@ -# this is used to allow custom dispatches to `_http_request` +# Used to allow custom dispatches to `_http_request` abstract type AbstractBackend end """ diff --git a/test/patch.jl b/test/patch.jl index cb9156105c..dddb1c6ce8 100644 --- a/test/patch.jl +++ b/test/patch.jl @@ -56,7 +56,7 @@ end function _aws_http_request_patch(response::HTTP.Messages.Response=_response()) - return @patch AWS._http_request(::AWS.HTTPBackend, request::Request) = response + return @patch AWS._http_request(::AWS.AbstractBackend, request::Request) = response end _cred_file_patch = @patch function dot_aws_credentials_file() @@ -73,7 +73,7 @@ _web_identity_patch = function (; session_token="web_session_token", role_arn="arn:aws:sts:::assumed-role/role-name", ) - @patch function AWS._http_request(::AWS.HTTPBackend, request) + @patch function AWS._http_request(::AWS.AbstractBackend, request) params = Dict(split.(split(request.content, '&'), '=')) creds = Dict( "AccessKeyId" => access_key, From 22974c952c1dd51796ce4da55c41b3ee9659fe48 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Mon, 19 Jul 2021 15:20:57 +0200 Subject: [PATCH 09/11] test with custom backend --- test/AWS.jl | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/test/AWS.jl b/test/AWS.jl index 22ca1219e6..45c5752930 100644 --- a/test/AWS.jl +++ b/test/AWS.jl @@ -351,6 +351,14 @@ end end end +struct TestBackend <: AWS.AbstractBackend + param::Int +end + +function AWS._http_request(backend::TestBackend, request) + return backend.param +end + @testset "HTTPBackend" begin request = Request( service="s3", @@ -362,7 +370,7 @@ end # No default options @test isempty(AWS._http_request(request.backend, request)) - # We can pass options via the backend + # We can pass HTTP options via the backend custom_backend = AWS.HTTPBackend(Dict(:connection_limit => 5)) @test custom_backend isa AWS.AbstractBackend @test AWS._http_request(custom_backend, request) == Dict(:connection_limit => 5) @@ -376,22 +384,23 @@ end custom_backend = AWS.HTTPBackend(Dict(:pipeline_limit => 5)) @test AWS._http_request(custom_backend, request) == Dict(:pipeline_limit => 20) end + + request.backend = TestBackend(2) + @test AWS._http_request(request.backend, request) == 2 # Let's test setting the default backend + prev_backend = AWS.DEFAULT_BACKEND[] try - AWS.DEFAULT_BACKEND[] = AWS.HTTPBackend(; connection_limit = 3) + AWS.DEFAULT_BACKEND[] = TestBackend(3) request = Request( service="s3", api_version="api_version", request_method="GET", url="https://s3.us-east-1.amazonaws.com/sample-bucket", ) - @test request.backend.http_options == Dict(:connection_limit => 3) - apply(Patches._http_options_patch) do - @test AWS._http_request(request.backend, request) == Dict(:connection_limit => 3) - end + @test AWS._http_request(request.backend, request) == 3 finally - AWS.DEFAULT_BACKEND[] = AWS.HTTPBackend() + AWS.DEFAULT_BACKEND[] = prev_backend end end From 4760f18e5de8904cd609af0241e9ce73b11b5be2 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Mon, 19 Jul 2021 15:25:45 +0200 Subject: [PATCH 10/11] populate `DEFAULT_BACKEND` in `__init__` --- src/AWS.jl | 5 +++++ src/utilities/request.jl | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/AWS.jl b/src/AWS.jl index c5c60beb30..e205dff8ad 100644 --- a/src/AWS.jl +++ b/src/AWS.jl @@ -353,4 +353,9 @@ function (service::RestJSONService)( return submit_request(aws_config, request; return_headers=return_headers) end +function __init__() + DEFAULT_BACKEND[] = HTTPBackend() + return nothing +end + end # module AWS diff --git a/src/utilities/request.jl b/src/utilities/request.jl index ec8270afc1..dc3138ae4d 100644 --- a/src/utilities/request.jl +++ b/src/utilities/request.jl @@ -16,7 +16,8 @@ function HTTPBackend(; kwargs...) isempty(kwargs) ? HTTPBackend(LittleDict{Symbol, Any}()) : HTTPBackend(LittleDict(kwargs)) end -const DEFAULT_BACKEND = Ref{AbstractBackend}(HTTPBackend()) +# populated in `__init__` +const DEFAULT_BACKEND = Ref{AbstractBackend}() Base.@kwdef mutable struct Request service::String From 73f7217fe30bb918300f4522c18c5dce415ad60c Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Mon, 19 Jul 2021 16:24:17 +0200 Subject: [PATCH 11/11] add test with `backend` keyword argument --- test/AWS.jl | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/AWS.jl b/test/AWS.jl index 45c5752930..6a1946ab8f 100644 --- a/test/AWS.jl +++ b/test/AWS.jl @@ -388,6 +388,15 @@ end request.backend = TestBackend(2) @test AWS._http_request(request.backend, request) == 2 + request = Request( + service="s3", + api_version="api_version", + request_method="GET", + url="https://s3.us-east-1.amazonaws.com/sample-bucket", + backend = TestBackend(4) + ) + @test AWS._http_request(request.backend, request) == 4 + # Let's test setting the default backend prev_backend = AWS.DEFAULT_BACKEND[] try