State should not depend on the API server

[Nate Finch]

I have been in situations in past jobs where adding one field to one
logical entity required changing the signature of a half dozen structures &
functions that were just copying data back and forth for little to no
benefit.  This is a pain in the ass, error prone, and should be avoided.

One of the things I like best about Go is that structs can be "just data".
 How you interact with the data can be different from layer to layer, but
there's no reason why we need different structs with the same layout in
different packages just for the illusion of separation of concerns.  If you
modify one, you have to modify all of them, thus you might as well just
have one they all use.  At least then it's clear which places in the code
depend on this logical data structure, rather than having to try to grep
for everywhere we copy the data from one structure to another.  This is why
having the api/params leaf package is a good idea - it's just data.  Any
package can use those structs to layout their data in a way that other
packages can understand.

I don't see how having different identical structs that are updated
simultaneously in any way prevents any problems with compatibility.

If they're different structs, they cannot possibly actually represent the
same data on both sides of the wire, can they?  One or both of the structs
must have information that is not present on the wire and that information
should be in a different type (which may embed the wire-data-struct).

Maybe I'm missing something from the proposal, but it seems like this just
adds busywork without actually solving any problems that weren't caused by
incorrect use of the code in the first place.

Separation of logic is absolutely a good thing.  Separation of data is not
nearly so useful.

On Thu, Sep 11, 2014 at 3:46 AM, roger peppe <roger.peppe at canonical.com>
wrote:

> 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.
>
> --
> Juju-dev mailing list
> Juju-dev at lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/juju-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/juju-dev/attachments/20140911/a0fc9a51/attachment.html>