Daniel Smith

@KarlKFI No worries, I totally understand the frustration, I just thought some explanation might make it sting less, I didn't mean to give you a hard time :)

@originalavalamp To be fair, I really like having a watch API. And I’ve built a career around using it to build platforms and integrations. So it’s not bad as a whole. There’s just this pebble in my shoe that I keep stubbing my toe on and can’t fix it, because it would break the whole ecosystem

Don't copy the #Kubernetes watch.Interface. It's bad. Instead of a separate Stop() method, force the event consumer to pass the means of stopping as an argument to the method that returns the event channel, like ResultChan(context.Context) or ResultChan(<-chan struct{}).

@KarlKFI No worries, I don't care about the code a decade later, I'm just reacting to the lack of nuance, which I guess is a silly thing to do on Twitter :)

@originalavalamp Anyway, I'm sorry if it came off as attacking you. That wasn't my intention. I just didn't have a lot of characters to be more diplomatic in the first tweet.

@KarlKFI Yes, producer can't send on a closed channel, but I don't think you can deduce your position from that fact. The Stop() method needn't (likely shouldn't) be the thing that closes the channel.

@originalavalamp Go has always panicked when you try to send on a closed channel. But I’m not trying to say “shame on you”. I’m just trying to say “don’t copy this pattern”.

@KarlKFI I'm not really defending the watch.Interface, which I'm sure can be improved, I'm mostly just observing that code isolated from the rest of the system generally isn't objectively bad or good, but is rather fit for a purpose or not.

@KarlKFI Do you have a 2014 or earlier source on that convention? :) But even if you do, it wouldn't have mattered, I learned go in isolation prior to go1.0, and the rest of the team hadn't used go at all prior to Kubernetes.

@KarlKFI Most interface changes trade one such tempting misuse for another. And at the time, it was certainly not worth spending days or weeks to make a bullet-proof interface.

@KarlKFI Sounds like FakeWatcher was written to only exercise a certain set of scenarios and things spiraled from there. That happens a lot with code.

@KarlKFI Few interfaces necessarily lack that property.

@originalavalamp It also makes how you use it dependent on how it’s implemented, which makes for lots of fun little subtle bugs.

@KarlKFI I don't believe my implementation had such a problem, but I haven't looked in a long time, maybe it grew one. The observation was that both sides have to do the same cleanup no matter which side issues the stop, so it wasn't necessary to have separate stop functions for both.

@originalavalamp If either side can call stop, then it doesn’t function as a method for ensuring bi-directional cleanup, and allows race conditions between the stop channel and result channel, leading to non-deterministic results.

@KarlKFI I wrote that before context was a thing. The intention was either side could call stop and it was safe to call multiple times. Once stopped there may still be items to drain, but generally that wasn't important because the client just started over with the last RV it saw anyway.

A separate Stop() method makes it ambiguous who should call stop, when they should do it, and how many times they can do it. For the watch pattern to work, you need bi-directional signaling to allow cleanup on both sides.

@IanColdwater But to be extra clear, apiserver definitely existed, even in the first prototype.

@IanColdwater (the client library was scrambled because I wasn't clear on what called what when I split the prototype into separate packages. It was many years before that mistake got fixed.)

@bwplotka @RiRa12621 @the_sttts Everything is tradeoffs, keeping working code working is (and should be) their top priority. What I said wasn't meant as a criticism of the go team. Personally I love the generics! When it is a good idea to use them is also a tradeoff, of course.

@RiRa12621 @the_sttts Well explained by @originalavalamp - consistency with old stuff vs less surprising options within local context: twitter.com/originalavalam… There is some wisdom here-tons of snowflakes can also cause programming language to kill itself over time. Also just don't use generics (:

Same tweet without any sarcasm: type X[T comparable] ... type Y X[any] How can "any" satisfy this if it's not comparable? I would consider myself a serious Go dev, but cannot answer it. Anybody?

@the_sttts Yeah it is horrible to do this to the spec, but the error happened prior to templates and backwards compatibility is more important.

@originalavalamp Yes. The blog post mentions that go.dev/blog/comparable. And to avoid inconsistency of their model, any cannot implement comparable but satisfies it as a type constraint.

@liggitt @ibuildthecloud @dims I wasn't going to mention names--there's more than one cook in that kitchen--but yes. I think apiserver internal code should be judged more on whether it does what it needs to than on being easy to reuse etc. It kind of froze half finished because k8s got too big to take risks.

@ibuildthecloud @originalavalamp @dims Original API server wasn't structured that way, but deads2k did a massive series of refactors in 2016-2017 as part of admission and custom resource additions that shifted to that structure. And yes, he was a Java developer in a former life.

glad we are all new comers in k8s !! Don't feel bad folks.

@ibuildthecloud @dims No. The initial package layout was due to me and I have no Java experience. (While I had more go experience than others on the team at that time, it was all idiosyncratic.) The people with Java experience got on board a bit later. I don't recall when the 'util' packages appeared.

@dims I kid you not, util is more acceptable than utils. I don't make the rules. But also realize k8s code started as Java with Go syntax.

@the_sttts @kubernetesio 100% coincidence. We were looking at them in go struct order so we had no idea

How long did the very early @kubernetesio developers think about the top-level keys to find words that appear in that order? apiVersion kind metadata spec status This cannot be a coincidence, can it? #historyofkube

@KarlKFI @ibuildthecloud if an owner changed, it's not a no-op

@originalavalamp @ibuildthecloud But is it a no-op if you specify it and someone else previously owned it and you have force=true, even if the value didn’t change?

k8s can do anything and everything.... except an updated timestamp. That's madness, that would never scale.