<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Feb 10, 2015 at 5:44 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:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">So, I think you *do* need conversion code specific to state -- the<br>
methods exported from state should be in terms of the storage types,<br>
because they're the language we've chosen to model storage, so there<br>
needs to be conversion code in state (or possibly in an internal<br>
package -- but either way there's no reason to expose it to anyone<br>
else).<br></blockquote><div><br></div><div>Not sure I follow what you're saying about "in terms of the storage types", so</div><div>I'll describe how it is at the moment.</div><div><br></div><div>The types are in three places:</div><div>  - juju/juju/storage</div><div>  - juju/juju/apiserver/params</div><div>  - juju/juju/state</div><div><br></div><div>There's a storage.Volume, a params.Volume, and a state.Volume. state.Volume</div><div>is actually an interface, but there's a VolumeInfo which ~correlates to the other</div><div>struct types.</div><div><br></div><div>I think the conversion from params<->state all needs to be in apiserver/<something>.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
WRT params, though, we already have two groups of client packages, so<br>
the same forces don't quite apply: and I still prefer that we *not*<br>
import other packages into params [0]. How about a single<br>
params/convert package?<br></blockquote><div><br></div><div>I could live with it, but I don't like the idea of adding dependencies to every domain</div><div>for every facade that uses it.</div><div><br></div><div>Cheers,</div><div>Andrew</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Cheers<br>
William<br>
<br>
[0] this is because the temptation to use a storage.Foo in place of a<br>
params.Foo has historically been overwhelming (and while it's *very<br>
dangerous* to do that it's hard to make the danger sufficiently<br>
obvious to dissuade distracted devs from doing so).<br>
<div class=""><div class="h5"><br>
On Tue, Feb 10, 2015 at 11:07 AM, Andrew Wilkins<br>
<<a href="mailto:andrew.wilkins@canonical.com">andrew.wilkins@canonical.com</a>> wrote:<br>
> Hi all,<br>
><br>
> Anastasia raised an issue in <a href="http://reviews.vapour.ws/r/885/" target="_blank">http://reviews.vapour.ws/r/885/</a> about how to<br>
> cut down on struct conversions between params, state, and domain packages.<br>
> In this case we're talking about storage. The following API server facades<br>
> currently participate in storage:<br>
>    - client<br>
>    - storage<br>
>    - provisioner<br>
>    - diskmanager (to be renamed, this lists block devices on machines)<br>
>    - diskformatter<br>
>    - storageworker (to be renamed, this is the dynamic storage provisioner)<br>
><br>
> Each facade have some overlap in dealing with storage entities, e.g. the<br>
> diskformatter and diskmanager each need to deal with block devices. This<br>
> leads to much duplication of struct copying/conversion code when toing and<br>
> froing between state and clients.<br>
><br>
> I don't want to go adding conversion code to the params, state or storage<br>
> packages, as they really shouldn't have dependencies on each other. Does<br>
> anyone have a good idea about where to put this common functionality? Maybe<br>
> "api/common/storage", "apiserver/common/storage"? Does not appeal, but I<br>
> can't think of a better option.<br>
><br>
> Cheers,<br>
> Andrew<br>
><br>
</div></div><span class=""><font color="#888888">> --<br>
> Juju-dev mailing list<br>
> <a href="mailto:Juju-dev@lists.ubuntu.com">Juju-dev@lists.ubuntu.com</a><br>
> Modify settings or unsubscribe at:<br>
> <a href="https://lists.ubuntu.com/mailman/listinfo/juju-dev" target="_blank">https://lists.ubuntu.com/mailman/listinfo/juju-dev</a><br>
><br>
</font></span></blockquote></div><br></div></div>