Add HTTPServer so we don't vend BlueSocketHTTP #26
Conversation
@swift-server/stakeholders any opinions of this as a very early implementation of an API to create/start the HTTP Server itself? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Sources/HTTP/HTTPServer.swift
Outdated
public protocol HTTPServer : class { | ||
|
||
/// Start the HTTP server on the given `port`, using `handler` to process incoming requests | ||
func start(port: Int, handler: @escaping HTTPRequestHandler) throws |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bind
also accepts an address name. should we support that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Golang's net/http/Server has an Addr string
field which seems to encapsulate both the address name and port. I'm not sure whether there's a standard format for this that we could/should use?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice 👏 !
Sources/HTTP/HTTPServer.swift
Outdated
/// A basic HTTP server. Currently this is implemented using the BlueSocket | ||
/// abstraction, but the intention is to remove this dependency and reimplement | ||
/// the class using transport APIs provided by the Server APIs working group. | ||
public class SimpleHTTPServer { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure to understand the purpose of this public class. If it is something that devs are supposed to use, wouldn't it be better to call the protocol HTTPServerType
and this class just HTTPServer
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to have a public constructor. And agreed - I'd prefer the concrete server class to be just HTTPServer
I've updated the PR to rename |
Question for people: do we want to mark |
8be7338
to
b897246
Compare
Personally, I not really sure what universal API for initializing http servers should exist. Of course it can cover simple handmade http servers (maybe even both sync and async), but advanced configuration will be problematic and it can't cover http servers what already exist (think about uwsgi, nginx, httpd, ...) - they usually start using configuration file and then pass some control to application. May be |
The aim of the |
Out of the box http server is good feature but it should be placed in a separate module to not inject additional dependencies into API's one. Some more questions:
|
Yes, the The intent is to provide an async server out of the box. Its hard to argue that the current and future concurrency model for Swift is not async - Dispatch is provided as part of the existing Swift toolchains, and large swathes of the available APIs are async based. Additionally both the Swift 5 manifesto and the recent async/await proposal points to that as the ongoing direction. |
I agree with that, if you are willing to add 'default' ;-) What is wrong with using Swift to write Apache modules, Android stuff, Outlook plugins, or whatever. Nothing. There is little use in tying Swift to a specific setup.
And those are super limited. Even the Linux implementation is, well, you probably know the issues much better than I do ;-) Dispatch is in fact a major issue with secondary toolchains (BSD? Raspi? Other IoT etc). Also specifically for Dispatch, that Tesla^d^d^d^d Google guy on Concurrency in Swift:
That is something which is bothering me as well (but may not affect the core choice for async, i.e. one could use libuv too). In general my opinion is that having a basic http server lib is nice in any case (I really don't expect that serious server frameworks are going to use this for plenty of reasons). So maybe lets just get this done, but lets try to make it so, that other implementations can still use the same or similar API. |
Thanks Helge. I'm going to merge this as-is: the remaining questions are around the need for the |
but from what I can tell the current implementation is synchronous and blocking. Will this change soon? I think we should definitely do that before pitching to swift-evo. |
@weissi that's the intent, yes - any help you can provide would be greatly appreciated! |
This PR adds a simple
HTTPServer
protocol, and an implementation of it calledSimpleHTTPServer
. The purpose of this is so that people can use this package to run an HTTP server without having to refer to BlueSocket related types. In the future, we want to remove all of those from the package and we can't do that if we've vended them as public API.It also renames the
WebApp
andWebAppContaining
types (which didn't have much support) toHTTPRequestHandler
andHTTPRequestHandling
.