[BREAKING]: Modernize core server code and improve overall ergonomics #854
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
There have been a few issues (#587, #563) around the overall ergonomics of using `HTTP.listen` and the code hasn't had a good comb-through in a while. I used the core golang [server code](https://github.com/golang/go/blob/master/src/net/http/server.go) as a reference to see what kinds of things they allowed in terms of configuration, functionality, and overall ergonomics. The changes proposed here include: * New `HTTP.listen!` non-blocking version of `HTTP.listen` that returns a `Server` object * `Server` object supports `wait`, `close`, and `forceclose`; `close` initiates a "graceful" shutdown where active connections are waited for until they are finished being processed; `forceclose` just force closes all open connections. * The combination of `HTTP.listen!` + `Server` mean we have a much more convenient and simple way to interact with a running server. * In addition, when calling the provided handler function `f`, we use `Base.invokelatest(f, stream)` which gives the nice property of allowing Revise to "update" a live server by reflecting edits made to the handler/middleware stack. * Removed `reuse_limit` and `rate_limit` features since they were either problematic in their implementation or not really that useful * Tried to clean up the verbose logging story, though there's more to do here * Tried to clean up some core listenloop logic, though there's more to do here Otherwise, the core `HTTP.listen` function remains mostly unchanged, except for the changes to supported keyword arguments. Still need to update some tests, docs, and comb-through the `handle_transaction` function, but wanted to put this up now in case others are interested in taking a look.
This was referenced Jun 18, 2022
quinnj
changed the title
Codecov Report
@@ Coverage Diff @@
## master #854 +/- ##
==========================================
- Coverage 80.53% 79.78% -0.75%
==========================================
Files 36 36
Lines 2825 2849 +24
==========================================
- Hits 2275 2273 -2
- Misses 550 576 +26
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
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.
There have been a few issues (#587, #563) around the overall ergonomics
of using
HTTP.listenand the code hasn't had a good comb-through in a while.I used the core golang server code
as a reference to see what kinds of things they allowed in terms of configuration,
functionality, and overall ergonomics.
The changes proposed here include:
HTTP.listen!non-blocking version ofHTTP.listenthat returns aServerobjectServerobject supportswait,close, andforceclose;closeinitiates a "graceful"shutdown where active connections are waited for until they are finished being processed;
forceclosejust force closes all open connections.HTTP.listen!+Servermean we have a much more convenient and simpleway to interact with a running server.
f, we useBase.invokelatest(f, stream)which gives the nice property of allowing Revise to "update" a live server by reflecting edits
made to the handler/middleware stack.
reuse_limitandrate_limitfeatures since they were either problematic in theirimplementation or not really that useful
Otherwise, the core
HTTP.listenfunction remains mostly unchanged, except for the changes to supportedkeyword arguments. Still need to update some tests, docs, and comb-through the
handle_transactionfunction, but wanted to put this up now in case others are interested in taking a look.