search

ahmetb / go-httpbin

http://httpbin.org endpoints for your Go tests
https://godoc.org/github.com/ahmetalpbalkan/go-httpbin
Apache License 2.0
124 stars 26 forks source link

Add /brotli endpoint #20

Closed gaul closed 5 years ago

gaul commented 5 years ago

Parity with http://httpbin.org/

ahmetb commented 5 years ago

Thanks for the PR, it looks good to me. So far we managed to keep this package vanilla-Go. This would be the first dependency that we accept.

I'm not entirely sure if we should start doing this. Do you have use case or do you think there's enough demand for /brotli? We already don't have 1:1 parity with httpbin.org right now (they definitely have more endpoints) so I need some convincing.

gaul commented 5 years ago

I want to add Brotli testing to compy to verify some behavior in barnacs/compy#47 and backfill our coverage. We currently use go-httpbin since this integrates most readily but I suppose we could stub out some tests which run against the httpbin.org service. I appreciate that taking a C dependency would impose a new requirement on all users and if this scopes out of the project currently I will understand.

ahmetb commented 5 years ago

Can you potentially use httpbin.GetMux() (which is of type mux.Router) and register /brotli yourself in your code? This way you can extend go-httpbin.

If all you're testing is GET /brotli, then you probably don't even need go-httpbin in the first place.

gaul commented 5 years ago

This seems like a good workaround if this PR doesn't merge. However, we do use go-httpbin for all our other tests so it would be nice to have this mainline.

ahmetb commented 5 years ago

The idea of cgo is scaring me a bit. The only external build dependency today is github.com/gorilla/mux and that's kind of needed to solve URL handling cleanly.

Since it's extensible and you can patch it with something like registerBrotli(mux) I'm tempted to close this –if the request comes up independently (and perhaps we have a Go-native Brotli implementation by then), I can accept.

Sorry about this.

gaul commented 5 years ago

I'm tempted to close this –if the request comes up independently (and perhaps we have a Go-native Brotli implementation by then), I can accept.

No worries and thank you for suggesting a workaround!