groupcache icon indicating copy to clipboard operation
groupcache copied to clipboard

http pool basePath getter and/or setter?

Open dvirsky opened this issue 10 years ago • 8 comments

I'm building a server that uses groupcache, and I would like to be able to conveniently wrap the http pool's handler inside a mux (specifically to add a monitoring status call on the server).

I simply thought of adding a handler func that forwards everything under /_groupcache/ to groupcache's handler. But this will fail if ever the hard coded base path will change.

So I thought of adding at least a getter, if not a setter for the base path. I understand the consistency problem with adding a setter. Will a pull request for GetBasePath() and/or SetBasePath() be accepted?

dvirsky avatar Feb 11 '14 10:02 dvirsky

Can't you just wrap the handler with http.StripPrefix/

nf avatar Feb 13 '14 23:02 nf

The way I understand it no, but I'd gladly stand corrected. Groupcache handles everything by default, and panics if that doesn't begin with its basePath (/_groupcache), that is not configurable (and has the comment: // TODO: make this configurable?), is not exported and doesn't have a getter.

I just hard coded that into my code, but you know, it feels wrong :)

dvirsky avatar Feb 13 '14 23:02 dvirsky

I wouldn't be opposed to adding these methods:

BasePath() string
SetBasePath(p string)

nf avatar Feb 13 '14 23:02 nf

Alright, I'll gladly do this and add do a PR.

dvirsky avatar Feb 14 '14 00:02 dvirsky

@nf wouldn't it be a tad more elegant, instead of adding a setter to simply add in http.go (67):

func NewHTTPPool(self, basePath string) *HTTPPool  {
...

I mean, you wouldn't change the basePath during serving.

dvirsky avatar Feb 14 '14 10:02 dvirsky

Yep, probably.

nf avatar Feb 15 '14 22:02 nf

Do that :-)

nf avatar Feb 15 '14 22:02 nf

I just hit this, I'd even be happy if defaultBasePath was DefaultBasePath so that muxer registration wasn't by a copy of the constant.

shwoodard avatar Feb 11 '16 04:02 shwoodard