Add HTTPClientRequest
#509
Conversation
dnadoba
added
🔨 semver/patch
| @available(*, deprecated, message: "no need to manually specify `length` because we automatically use `bytes.count` as the `length`") | ||
| static func bytes<Bytes>( | ||
| length: Int, | ||
| _ collection: Bytes | ||
| ) -> Self where Bytes: RandomAccessCollection, Bytes.Element == UInt8 { | ||
| return .bytes(collection) | ||
| } |
There was a problem hiding this comment.
This is an interesting idea... It pushes people into using the right thing. However I wonder if it is a good idea to introduce new API, that we deprecate from the get go. @Lukasa wdyt?
There was a problem hiding this comment.
I'd be more inclined to consider marking the method @available(*, unavailable) and then not giving it an implementation beyond fatalError
There was a problem hiding this comment.
I have tried @available(*, unavailable) but then the swift compiler will just choose the overload where Bytes: Sequence:
Even adding
@_disfavoredOverload to the Sequence overload doesn't help.
I think what we could also do is marking the method as deprecated and just adding a fatalError with a useful message. Then user will get a compiler warning and a runtime error. WDYT?
| self.init(.byteBuffer(byteBuffer)) | ||
| } | ||
|
|
||
| static func bytes<Bytes>( |
There was a problem hiding this comment.
Can we make this method @inlinable?
| }) | ||
| } | ||
|
|
||
| static func bytes<Bytes>( |
There was a problem hiding this comment.
Can we make this method @inlinable?
| /// `RandomAccessCollection` because it already provide a property `count` to get the length in O(**1**). | ||
| /// - Note: `length` is ignored in favour of `bytes.count` | ||
| @available(*, deprecated, message: "no need to manually specify `length` because we automatically use `bytes.count` as the `length`") | ||
| static func bytes<Bytes>( |
There was a problem hiding this comment.
Can we make this method @inlinable?
| length: Int, | ||
| _ collection: Bytes | ||
| ) -> Self where Bytes: RandomAccessCollection, Bytes.Element == UInt8 { | ||
| return .bytes(collection) |
There was a problem hiding this comment.
If we're going to offer this overload, can we at least precondition that the length matches the count of the collection?
| @available(*, deprecated, message: "no need to manually specify `length` because we automatically use `bytes.count` as the `length`") | ||
| static func bytes<Bytes>( | ||
| length: Int, | ||
| _ collection: Bytes | ||
| ) -> Self where Bytes: RandomAccessCollection, Bytes.Element == UInt8 { | ||
| return .bytes(collection) | ||
| } |
There was a problem hiding this comment.
I'd be more inclined to consider marking the method @available(*, unavailable) and then not giving it an implementation beyond fatalError
| return .bytes(collection) | ||
| } | ||
|
|
||
| static func stream<SequenceOfBytes>( |
There was a problem hiding this comment.
Can we make this method @inlinable?
| return body | ||
| } | ||
|
|
||
| static func stream<Bytes>( |
There was a problem hiding this comment.
Can we make this method @inlinable?
Yes |
HTTPClientRequestandHTTPClientRequest.Bodyas described in the async/await proposal with some modifications:.bytes(_:)and.stream(_:)fromHTTPClientRequest.Bodyaccept an additional optionallengthparameter to specify thelengthin advance. This will primarily be used to set aContent-Lengthheader.HTTPClientRequest.Body.bytes(..)that accepts aRandomAccessCollectionwhich automatically uses thecountproperty from the given collection as the body length.Note that everything is still internal. This allows us to merge async/await components incrementally while still being able to release new versions from main.