State should not depend on the API server

[roger peppe]

On 11 September 2014 06:40, John Meinel <john at arbash-meinel.com> wrote:
>> ...
>> I have thought for a while that, rather than writing more error-prone code
>> (at > 17k LOC, surely the API code is big enough as it is?), it would
>> be good to create a tool that helps us with the underlying problem -
>> incompatible changes made to marshaled types.
>>
>> This would not be too hard - the Go language already has such a tool
>> (see http://golang.org/cmd/api/ ), although it is not currently generally
>> applicable.
>>
>> This could make it possible to provide a much stronger assurance about
>> compatibility, including compatibility between types not defined inside
>> juju-core itself, while keeping the code natural and simple.
>>
>>   cheers,
>>     rog.
>>
>
> So that is interesting, and it might be fruitful to explore. I'll just note
> that it is actually a pretty small (and reasonably obvious) part of API
> compatibility. It *doesn't* tell you
>
> 1) If passing an empty string suddenly has a different meaning (see
> Client.ServiceSet where we used to treat "" as meaning reset-to-default vs
> Client.NewServiceSetForAPI where we wanted to actually treat "" as meaning
> set this value to the empty string.)
>
> 2) Returning a new value (Machine.Jobs(), we want to add new Jobs for new
> behaviors and don't want to confuse old agents until they have finished
> upgrading).

Yes, I agree. There are aspects of compatibility that
cannot be checked automatically, and we always need
to be careful when changing semantics. I don't
think there's any way around that.

> So while something like the above could tell you "has the shape of the API
> changed", and what I really like about it is that if it had a collapsed view
> (like a simple text representation), it is easy to keep that around in the
> source tree and use it for reference.

Yes, that's a nice thing about the Go api command, which does
use a simple text representation.

> However, you really do need tests for (1) and (2). (1) we actually broke
> accidentally because we changed the code inside of state/*, and then had to
> go back and retrofit the API to use a new code path to preserve
> compatibility.

Yup.

> And if you need tests, you really still need 2 types to represent the
> different versions, and if you already need those types, then I would
> recommend having a regular structure for them.

Here's where I don't agree. I don't think, for example, that it's worth
creating a new type just because its Jobs field can take on a value
that it could not previously have. We might need a new version of the API, sure
(although even that might be arguable in this case - agents just ignore
unknown job types), but defining a new type seems like overkill to me.

Even when we do need to make a new type, I don't really see why
having a different type at each level will help us. One will probably
need to make the new type at every level too, so having the
differently defined types isn't a great benefit over having
a single changed type.

There will of course be cases when we *do* want different types
at different levels, but that's easy to do on a case-by-case basis.
I suggest using appropriate types at the different levels
*as required by those levels*, rather than expending developer time and
generating extra runtime garbage by copying identical representations
of the same data around for every single type at every level.

  cheers,
    rog.