feat: implement send function #980
Conversation
| var nioRequest = AsyncHTTPClient.HTTPClientRequest(url: url.absoluteString) | ||
| nioRequest.method = method | ||
|
|
||
| for header in request.headers.headers + config.defaultHeaders.headers { |
There was a problem hiding this comment.
What if a header is set in the request AND in defaults?
Is the request's value used, the default's, or are both headers set? (HTTP is okay with two headers with the same keys and/or values)
There was a problem hiding this comment.
Addressed. Header in request will be preferred and overwrite if its in default as well.
| httpMetricsUsage.connectionsLimit = 0 // TODO: Get from AsyncHTTPClient configuration | ||
| do { | ||
| let timeout: TimeAmount = .seconds(Int64(config.socketTimeout)) | ||
| let nioResponse = try await client.execute(nioRequest, timeout: timeout) |
There was a problem hiding this comment.
lol is this really all it takes? Will it also do HTTP/2 bidi streaming out of the box for us?
There was a problem hiding this comment.
Yup :) All integration tests are passing for me!
There was a problem hiding this comment.
How is performance? We should look at the tests where we run huge #s of S3 streaming ops at the same time
There was a problem hiding this comment.
I'll be doing performance testing for the whole implementation prior to merging the feature branch to main. So that answer is incoming! But I dont think we need to hold off on merging this to the feature branch until we get those numbers.
2 participants
Issue #
Description of changes
Scope
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.