Try to make backend configuration a little more robust.

[xenoscopic]

If there's no change to the backend config, then there's no need to fail the request on an active runner. Also, if the active runner is unused, then we can evict it and then update its config.

[xenoscopic]

@silvin-lubecki I think this path should handle your case without eviction (we can look at eviction disablement or configuration in a separate PR).

[silvin-lubecki]

The logic is good, but I think we should avoid using DeepEqual if possible. Looking at the inference.BackendConfiguration type, it's simple enough we can add an Equal method:

import "slices"

func (b *BackendConfiguration) Equals(other *BackendConfiguration) bool {
    return b.ContextSize == other.ContextSize && 
           slices.Equal(b.RawFlags, other.RawFlags)
}

[xenoscopic]

Is there a risk to DeepEqual? It feels like there's a bigger risk if someone forgets to update Equals when they add a new field.

[silvin-lubecki]

Good point about the maintenance risk! That's definitely a valid concern.

However, I think an explicit Equals method would be beneficial here for a couple of reasons:

Explicitness and readability: With only 2 fields (ContextSize and RawFlags), the comparison logic is clear. For such a simple struct, the Equals method is just 2 lines with slices.Equal.

Type safety: reflect.DeepEqual accepts any types and will silently return false for type mismatches, which could mask bugs during refactoring.

For the maintenance concern, we could add a focused test:

func TestBackendConfigurationEquals(t *testing.T) {
    base := &BackendConfiguration{ContextSize: 100, RawFlags: []string{"--flag"}}
    
    // Test each field difference
    different := *base
    different.ContextSize = 200
    require.False(t, base.Equals(&different))
    
    different = *base
    different.RawFlags = []string{"--other"}
    require.False(t, base.Equals(&different))
}

This way, adding a new field without updating Equals would likely be caught by the test.
That said, DeepEqual works perfectly fine for this use case too - what's your preference?

[xenoscopic]

I'll leave it @p1-0tr to decide since he's sort of the CODEOWNER here 😉. I tend to lean towards less code = less maintenance. I don't see a risk with differing types since everything here is statically typed.

[fiam]

Not sure if that helps, but it seems the type can be safely compared with ==.

Sorry, looked at the wrong thing. Definitely not comparable with ==.

[p1-0tr]

I dislike reflection as a matter of principle. But in this case I don't mind the DeepEquals, because it keeps the code simple and small, and it will likely disappear once we implement proper runner configuration management.

[silvin-lubecki]

Just to be clear, I won't block on this 😄 I think I join @p1-0tr on this, I raised it as a matter of principle. And if it's temporary, then LGTM ✅

[silvin-lubecki]

LGTM