Merge #401

401: provide hooks for custom backends r=omus a=ericphanson

Instead of adding a whole new backend as in #396, I thought it would be better to just start with an `AbstractBackend` to provide hooks to use alternate backends. This way we don't need to bump the Julia requirements to start trying out alternate backends downstream. It also provides a smaller incremental step than having AbstractBackends + a specific alternate backend in the same PR.

This also provides what is hopefully a useful feature: you could do
```julia
using AWS
AWS.DEFAULT_BACKEND[] = HTTPBackend(custom_http_options)
```
to specify custom HTTP options globally in your application (which can still be overwritten per-request); I would have found this useful when trying experiments like tweaking the pipelining and connection pooling settings.

In the same way, a Downloads.jl-based backend could pass a `Downloads.Downloader` along inside the backend object instead of requiring adding more fields to Request for the `downloader` like in #396.

Co-authored-by: Eric Hanson <[email protected]>

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

src/utilities/request.jl

@@ -1,3 +1,24 @@
+# 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.
+"""
+struct HTTPBackend <: AbstractBackend
+    http_options::AbstractDict{Symbol,<:Any}
+end
+
+function HTTPBackend(; kwargs...)
+    isempty(kwargs) ? HTTPBackend(LittleDict{Symbol, Any}()) : HTTPBackend(LittleDict(kwargs))
+end
+
+# populated in `__init__`
+const DEFAULT_BACKEND = Ref{AbstractBackend}()
+
 Base.@kwdef mutable struct Request
     service::String
     api_version::String
@@ -13,6 +34,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 +76,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,23 +163,25 @@ 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)
 
         if request.return_stream && request.response_stream === nothing
             request.response_stream = Base.BufferStream()
         end
 
-        return HTTP.request(
+        return @mock HTTP.request(
             http_stack,
             request.request_method,
             HTTP.URI(request.url),
             HTTP.mkheaders(request.headers),
             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

src/utilities/utilities.jl

@@ -69,6 +69,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
 

test/AWS.jl

@@ -351,6 +351,68 @@ 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",
+        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 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)
+
+        # 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
+
+    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
+        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 AWS._http_request(request.backend, request) == 3
+    finally
+        AWS.DEFAULT_BACKEND[] = prev_backend
+    end
+end
+
 @testset "_generate_rest_resource" begin
     request_uri = "/{Bucket}/{Key+}"
     args = Dict{String, Any}(

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(::AWS.AbstractBackend, request::Request) = response
 end
 
 _cred_file_patch = @patch function dot_aws_credentials_file()
@@ -103,4 +103,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