> On 10 Nov 2016, at 15:59, Ian Davis <m...@iandavis.com> wrote: > > Out of interest, do you have any examples of packages that perform a type > check to reject an operation with a response writer? Your implementation is > complex purely to address this scenario.
No, that was a bogus example. However, I do have an application that will misbehave if CloseNotify is not implemented or not implemented properly. And I think it’s also easy to imagine that it’s downright impossible to fake http.Hijacker if the underlaying ResponseWriter doesn’t support it. > I would think that 99+% of all type checks are to determine whether the > supplied object provides additional functionality that can be used. I'm > struggling to think of a situation where you would type check for the Flush > method and then refuse to use the main ResponseWriter methods if it were a > Flusher. I agree. But verifying if you’re part of the 99% or 1% may be a very complex task, especially when using 3rd party http related packages. So why would I want to worry about this invariant holding for a large amount of source code, if I can fix the problem with a comparably much smaller amount of boilerplate? Cheers Felix > > > On Thu, Nov 10, 2016, at 02:37 PM, Felix Geisendörfer wrote: >> There are a few problems with the Kubernetes implementation. >> >> The first thing I noticed is that they seem to have copied their code from >> the prometheus go client: >> >> https://github.com/prometheus/client_golang/blob/master/prometheus/http.go#L289 >> >> <https://github.com/prometheus/client_golang/blob/master/prometheus/http.go#L289> >> >> I don’t know if there was a copyright reassignment, but if not, it looks >> like they failed to honor the prometheus license. >> >> The next problem is that they don’t seem to handle the io.Reader>From >> interface. Looking at the commit history of the prometheus code, this is >> somewhat confusing, because they don’t seem to have had a version where this >> was missing. So the Kubernetes developer may have intentionally removed it. >> It’d be curious why. >> >> Last but not least, both the Kubernetes as well as the prometheus >> implementation only address 2 out of the 16 combinations of http.Flusher, >> http.CloseNotifier, http.Hijacker and io.ReaderFrom. Perhaps they determined >> that the go core is only using these 2 combinations. But that’s a bit too >> brittle of an invariant for me to rely on. Therefor my package does the >> painful thing and implements all 16 cases … >> >> Cheers >> Felix >> >>> On 10 Nov 2016, at 15:25, Ian Davis <m...@iandavis.com >>> <mailto:m...@iandavis.com>> wrote: >>> >>> >>> On Thu, Nov 10, 2016, at 02:21 PM, Felix Geisendörfer wrote: >>>> Yes, I thought about it :). >>>> >>>> Did you read the "Why this package exists” section of the README? >>> >>> Yes but obviously not closely enough :) >>> >>> Kubernetes takes a hybrid approach: >>> >>> https://github.com/kubernetes/kubernetes/blob/master/pkg/apiserver/metrics/metrics.go#L84 >>> >>> <https://github.com/kubernetes/kubernetes/blob/master/pkg/apiserver/metrics/metrics.go#L84> > -- You received this message because you are subscribed to the Google Groups "golang-nuts" group. To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
