Use Async calls from AsyncHTTPClient #555
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently streaming is not working
Codecov Report
@@ Coverage Diff @@
## 7.x.x #555 +/- ##
==========================================
- Coverage 77.17% 71.54% -5.64%
==========================================
Files 75 76 +1
Lines 7860 7847 -13
==========================================
- Hits 6066 5614 -452
- Misses 1794 2233 +439
... and 11 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
0xTim
reviewed
Jun 20, 2023
| XCTFail("Unexpected error: \(error)") | ||
| } | ||
| } | ||
| /* func testRequestStreaming() async throws { |
There was a problem hiding this comment.
Is this to be added back in or deleted?
There was a problem hiding this comment.
I should have made it clear streaming requests/responses don't work yet. They will the subject of the next few PRs. So I commented out the tests that would fail and will uncomment them as features are implemented.
| _ = try await responseTask | ||
| XCTAssertEqual(countAtomic.load(ordering: .relaxed), 0) | ||
| } | ||
| /* func testStreamingResponse() async { |
| let eventLoop = eventLoop ?? self.eventLoopGroup.any() | ||
| return try await self.execute(request: request, timeout: timeout, on: eventLoop, logger: logger, stream: stream).get() | ||
| ) async throws -> HTTPClientResponse { | ||
| preconditionFailure("Not supported") |
There was a problem hiding this comment.
Should we just remove this if it's not supported?
There was a problem hiding this comment.
It will get removed in the next PR. Currently it a stub used by another section of code
Comment on lines
+37
to
+41
| /* case .asyncSequence(let sequence, let length): | ||
| requestBody = .stream( | ||
| sequence, | ||
| length: length.map { .known($0) } ?? .unknown | ||
| ) */ |
There was a problem hiding this comment.
This is commented out while streaming requests don't work. It will be re-instated when I get them working
adam-fowler
force-pushed
the
async-http-client
branch
from
8931bb0
to
18766e6
Compare
|
Fixed the tests. There was a weird interaction between the test server which can only receive one request at a time and me not flush the response body when I don't expect one |
adam-fowler
added a commit
that referenced
this pull request
Jul 5, 2023
* Use async HTTPClient calls Currently streaming is not working * Comments * Extract HTTPBody out so it can be used in request * Flush body, when not expecting anything * Remove eventLoop from function comment headers
adam-fowler
added a commit
that referenced
this pull request
Jul 5, 2023
* Use async HTTPClient calls Currently streaming is not working * Comments * Extract HTTPBody out so it can be used in request * Flush body, when not expecting anything * Remove eventLoop from function comment headers
adam-fowler
added a commit
that referenced
this pull request
Jul 5, 2023
* Use async HTTPClient calls Currently streaming is not working * Comments * Extract HTTPBody out so it can be used in request * Flush body, when not expecting anything * Remove eventLoop from function comment headers
adam-fowler
added a commit
that referenced
this pull request
Jul 5, 2023
* Use async HTTPClient calls Currently streaming is not working * Comments * Extract HTTPBody out so it can be used in request * Flush body, when not expecting anything * Remove eventLoop from function comment headers
adam-fowler
added a commit
that referenced
this pull request
Jul 24, 2023
* Use async HTTPClient calls Currently streaming is not working * Comments * Extract HTTPBody out so it can be used in request * Flush body, when not expecting anything * Remove eventLoop from function comment headers
adam-fowler
added a commit
that referenced
this pull request
Jul 29, 2023
* Use async HTTPClient calls Currently streaming is not working * Comments * Extract HTTPBody out so it can be used in request * Flush body, when not expecting anything * Remove eventLoop from function comment headers
adam-fowler
added a commit
that referenced
this pull request
Aug 12, 2023
* Use async HTTPClient calls Currently streaming is not working * Comments * Extract HTTPBody out so it can be used in request * Flush body, when not expecting anything * Remove eventLoop from function comment headers
adam-fowler
added a commit
that referenced
this pull request
Dec 24, 2023
* Use async HTTPClient calls Currently streaming is not working * Comments * Extract HTTPBody out so it can be used in request * Flush body, when not expecting anything * Remove eventLoop from function comment headers
adam-fowler
added a commit
that referenced
this pull request
Apr 5, 2024
* Use async HTTPClient calls Currently streaming is not working * Comments * Extract HTTPBody out so it can be used in request * Flush body, when not expecting anything * Remove eventLoop from function comment headers
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This was complex. Trying to return back the AsyncSequence from AHC in a generic way until I found
AnyAsyncSequencein AHC. This simplified passing the body back from AHC through my own types.AnyAsyncSequenceas a generic method for passing AsyncSequences about.ByteBufferAsyncSequenceas a way to convert aByteBufferinto anAsyncSequenceofByteBufferslices.HTTPBodyto hold response payloads. This'll probably be also used in requests but I haven't got to that yet. If so I'll rename back toAWSPayloadfor some consistency. TheHTTPBodyno longer defines the type of data it holds (eg xml, json, raw data) this data is got from the serviceProtocolStreaming requests and response currently don't work, so related code and tests may be commented out