router/mux: Added an unhandler and helpers #157
Conversation
router.unhandler is used to safely remove handlers from the router. "Un" helper functions in mux exposed to "undo" each of the method types as well. Very simple test added on to existing test.
|
Would it be possible to have a map of routes and call Compile() after
|
|
The last line of unhandler is rt.setMachine(nil) which calls the compiler. |
|
Yep - noticed that. To be clearer: what does this look like with your PR Note: @zenazn calls the shots on this - but I think we might agree here. On Sat, Sep 5, 2015 at 7:42 AM M@ notifications@github.com wrote:
|
|
I added the helper methods to keep the same feel as the original code, as opposed to just one method where the bitmask is passed in. I can certainly nix the Un* functions if the author prefers. |
|
I don't want to encourage mutating live muxes—the synchronization required to do so safely is kind of annoying (originally Goji allowed mutating the middleware stack concurrently with live requests, but I've since backed those changes out because of how complicated it was. I've been meaning to do the same for handlers as well, even though that's a much more vanilla RCU). As an alternative, have you tried to implement a custom If that doesn't fit well with your use case, I'd love to hear more about what you're after—maybe there's another solution that's less intrusive. |
|
I originally wrote my own mux based on gorilla/mux, however I appreciated how goji elegantly passed the context around, and was very fast, so I rebased my project using it instead. I could do all of the work outside of the mux, sure, but having a dynamic multiplexer is kind of the point. Feel free to close out the PR if removing routes isn't of interest. |
|
Alright, fair enough. Would a layer of indirection be helpful here? I'm imagining something like this: goji.Get("/static", Route)
m := web.New()
goji.Handle("/*", func(c web.C, w http.ResponseWriter, r *http.Request) {
m.ServeHTTPC(c, w, r)
})
func reload() {
newm := web.New()
// add routes
m = newm
}(with some additional locking or |
|
I'm not sure how you'd do that without a stop-the-world pause and probably dumping all in-flight requests. By only messing with the Route array, I'm at most losing requests that were being served to now-dead endpoints (routes) which is fine (and handled), and the pauses for route changes are ~3-10 microsecond blinks, even on pretty sizable (subjective, I suppose) route arrays |
|
Here's a complete working example. All requests are completed using the package main
import (
"io"
"net/http"
"sync/atomic"
"github.com/zenazn/goji"
"github.com/zenazn/goji/web"
)
func main() {
a := web.New()
a.Handle("/*", func(w http.ResponseWriter, r *http.Request) {
io.WriteString(w, "Mux A")
})
b := web.New()
b.Handle("/*", func(w http.ResponseWriter, r *http.Request) {
io.WriteString(w, "Mux B")
})
var mux atomic.Value
mux.Store(a)
goji.Get("/a", func(w http.ResponseWriter, r *http.Request) {
io.WriteString(w, "Switching to A")
mux.Store(a)
})
goji.Get("/b", func(w http.ResponseWriter, r *http.Request) {
io.WriteString(w, "Switching to B")
mux.Store(b)
})
goji.Handle("/*", func(c web.C, w http.ResponseWriter, r *http.Request) {
mux.Load().(*web.Mux).ServeHTTPC(c, w, r)
})
goji.Serve()
} |
|
Yeah, using atomic saves some complexity I was imagining. Looks good to me, thank you! |
I needed to be able to remove routes on the fly (in conjunction with service discovery), so I added router.unhandler() to safely remove handlers from the router, and helper functions too. If there are any changes you'd prefer to see, please let me know.