On 26 May 2017 at 15:27, Carl Brown via swift-server-dev < swift-server-dev@swift.org> wrote:
Hi, Helge,
For purposes of building this prototype, I tried to stay with as many of
Johannes' original type signatures as I could. Therefore, I used an enum
for HTTPMethod because that's what Johannes had in his email from
The swift-server-dev Archives
Week-of-Mon-20170403/000422.html
I'm happy to get into that and other specifics as we go (and yes I added
the `RawRepresentable` to the enum because I'm more used to thinking in
"200" and "404" than .ok and .notFound, and you are right, a `UInt16` would
have been better for status (and maybe I shouldn't have done it at all) -
my bad).
But the question of the moment (for you and for the group) is - do you
think this is a good enough starting point for us to move it to the
Swift on Server · GitHub organization and start iterating via
GitHub issues & Pull Requests, or do you think we're still at the stage
where we need to have more email discussions about it before we're ready to
take that step?
-Carl
--
Carl Brown, Swift@IBM
Carl.Brown1@IBM.com (Work)
Austin, TX
On May 26, 2017, at 4:22 PM, Helge Heß via swift-server-dev < > swift-server-dev@swift.org> wrote:
On 26. May 2017, at 22:00, Chris Bailey via swift-server-dev < > swift-server-dev@swift.org> wrote:
HTTPRequest: https://github.com/carlbrown/HTTPSketch/blob/
master/Sources/HTTPSketch/HTTPRequest.swift
/// HTTP Methods handled by http_parser.[ch] supports
public enum HTTPMethod: String {
// case custom(method: String)
case UNKNOWN
Don’t restrict the API to a specific HTTP parser implementation. The
number of methods supported by http_parser changed multiple times in the
past (I myself added one to the official tree).
I still highly recommend using constants instead of an enum here. The
methods *will* change in the future. (At least BATCH is going to be added)
At the minimum do the custom(String) variant. UNKNOWN seems unacceptable
to me.
In the same run, I don’t understand why an enum is used for methods but
not for headers. Extra weird because the method is just a header in HTTP/2 …
I’d prefer constants for both.
func writeBody(data: DispatchData, completion: @escaping
(Result<POSIXError, ()>) -> Void)
Do we really want to expose POSIXError’s? I’d say have an own error type
for HTTP level errors and maybe something like this:
enum HTTPError {
case connectionClosed(cause: POSIXError?)
...
}
Also: DispatchData vs Data vs Ptr.
public enum HTTPResponseStatus: UInt, RawRepresentable {
/* The original spec used custom if you want to use a non-standard
response code or
have it available in a (UInt, String) pair from a higher-level web
framework.
Swift 3 doesn't like it - will revisit in Swift 4*/
//case custom(code: UInt, reasonPhrase: String)
Swift 3 does like it, you just can’t combine raw w/ extra values. That
just doesn’t make sense. I think the only thing you could hope for is that
this works on a raw:
case custom(code: UInt)
Also, why a `UInt`? Do you foresee 64-bit status codes? Should be UInt16.
Again: I’d prefer constants because status codes are defined as part of
RFCs and
will *definitely* change in the future. Something like
struct HTTPStatus {
let code : UInt16
static let `continue` : UInt16 = 100
etc.
}
That is future and source-compat proof.
public struct HTTPHeaders {
var storage: [String:[String]] /* lower cased keys */
var original: [(String, String)] /* original casing */
I don’t think the storage semantics should be part of the API. I’d prefer
a protocol here providing indexed access.
Finally, I still propose calling it HTTPRequestHead, HTTPResponseHead. The
body belongs to the request/response. Don’t care too much, but would be
better.
hh
_______________________________________________
swift-server-dev mailing list
swift-server-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-server-dev
_______________________________________________
swift-server-dev mailing list
swift-server-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-server-dev
