add initial stream bridge #964
Conversation
|
|
||
| do { | ||
| // Read a chunk from the stream (using default chunk size) | ||
| let data = try await stream.readAsync(upToCount: CHUNK_SIZE_BYTES) |
There was a problem hiding this comment.
Open to proposals on alternative methods of doing this. Intention behind this is to read a chunk at a time from our Smithy stream in order to write it to AsyncHTTPClient stream. CHUNK_SIZE_BYTES here is reused from the S3 chunking which I believe is 64 kb.
There was a problem hiding this comment.
I'm okay with 64kb for production use, but the value should probably be configurable, if only for testing
There was a problem hiding this comment.
A couple questions inline.
I also think we should have tests around this stream bridge, for an example see:
https://github.com/smithy-lang/smithy-swift/blob/main/Tests/ClientRuntimeTests/NetworkingTests/URLSession/FoundationStreamBridgeTests.swift
| let bufferedStream = BufferedStream() | ||
|
|
||
| // Start a background task to stream data from AsyncHTTPClient to BufferedStream | ||
| Task { |
There was a problem hiding this comment.
Where will this function get called from? If it is called from an async context, I would think it better to not create a Task here.
|
|
||
| struct AsyncIterator: AsyncIteratorProtocol { | ||
| private let stream: Smithy.Stream | ||
| private let allocator: ByteBufferAllocator |
There was a problem hiding this comment.
where do we get this allocator from?
There was a problem hiding this comment.
ByteBufferAllocator is a struct from NIOCore package utilized by async-http-client
|
|
||
| do { | ||
| // Read a chunk from the stream (using default chunk size) | ||
| let data = try await stream.readAsync(upToCount: CHUNK_SIZE_BYTES) |
There was a problem hiding this comment.
I'm okay with 64kb for production use, but the value should probably be configurable, if only for testing
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.