<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Feb 11, 2015 at 7:51 PM, William Reade <span dir="ltr"><<a href="mailto:william.reade@canonical.com" target="_blank">william.reade@canonical.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><span class="">On Tue, Feb 10, 2015 at 11:35 AM, Andrew Wilkins <<a href="mailto:andrew.wilkins@canonical.com" target="_blank">andrew.wilkins@canonical.com</a>> wrote:<br>> Not sure I follow what you're saying about "in terms of the storage types",<br>> so<br>> I'll describe how it is at the moment.<br>><br>> The types are in three places:<br>>   - juju/juju/storage<br><br></span>...which contains the types that represent "how juju thinks about storage" -- they're the actual model types.<br><br>>   - juju/juju/apiserver/params<br>>   - juju/juju/state<br><br>...which contain wire/db representations of the juju/juju/storage types to insulate the wire/db formats from changes to the above.<span class=""><br><br>> There's a storage.Volume, a params.Volume, and a state.Volume. state.Volume<br>> is actually an interface, but there's a VolumeInfo which ~correlates to the<br>> other<br>> struct types.<br>><br>> I think the conversion from params<->state all needs to be in<br>> apiserver/<something>.<br><br></span>I don't think that's right -- surely a state.Volume's VolumeInfo should actually be a storage.Volume? i.e. when the apiserver gets volume info over the wire it converts it to a storage.Volume at the earliest opportunity, and passes that into state where necessary, which only converts again when it needs to serialise; and similarly, in the other direction, the apiserver gets a storage.Volume directly out of state, and converts that to a params.Volume on the way out. Considering that our business logic is essentially smeared through state and apiserver, volume manipulation could happen in either layer; and we want that behaviour associated with the storage.Volume type, so we can move it between layers sanely; so we should have something like:<br><br><font face="monospace, monospace">           where               what<br></font><div><font face="monospace, monospace">    ============================================</font></div><div><font face="monospace, monospace">          <wire>            params.Volume<br>    -------------<convert here>-----------------<br>         apiserver          storage.Volume<br>    --------------------------------------------</font><div><font face="monospace, monospace">          state             storage.Volume<br>    -------------<convert here>-----------------</font></div><div><font face="monospace, monospace">           <db>             state.volumeDoc</font></div></div></div></blockquote><div><br></div><div>As discussed live, that sounds fine. We'll need a new storage.VolumeInfo type, which will contain information about a volume excluding its identity. A Volume is its identity (tag) and VolumeInfo.</div><div>In state we have a Volume entity interface which will return storage.Volume* type values. In general I'm trying to keep the operations on state.State, to make it all more easily testable. When it comes to Life, Remove, EnsureDead, etc., I'm not sure there's much choice without rewriting everything in state.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><span class=""><br>> I could live with it, but I don't like the idea of adding dependencies to<br>> every domain<br>> for every facade that uses it.</span></div><div><br></div><div>I think we really do want domain dependencies in any part of the apiserver that contains logic pertaining to that domain, even if the logic is simple conversion [0]. Without that, domain logic just gets attached to whatever type lies closest to hand and makes it harder to move it to the right place as the code evolves.</div><div><br></div><div>Cheers</div><div>William</div><div><br></div></div><div>[0] obligatory: <a href="http://thecodelesscode.com/case/97" target="_blank">http://thecodelesscode.com/case/97</a></div></div>
</blockquote></div><br></div><div class="gmail_extra">:)</div></div>