On not copy-pasting state and params struct conversions
William Reade
william.reade at canonical.com
Tue Feb 10 09:44:35 UTC 2015
So, I think you *do* need conversion code specific to state -- the
methods exported from state should be in terms of the storage types,
because they're the language we've chosen to model storage, so there
needs to be conversion code in state (or possibly in an internal
package -- but either way there's no reason to expose it to anyone
else).
WRT params, though, we already have two groups of client packages, so
the same forces don't quite apply: and I still prefer that we *not*
import other packages into params [0]. How about a single
params/convert package?
Cheers
William
[0] this is because the temptation to use a storage.Foo in place of a
params.Foo has historically been overwhelming (and while it's *very
dangerous* to do that it's hard to make the danger sufficiently
obvious to dissuade distracted devs from doing so).
On Tue, Feb 10, 2015 at 11:07 AM, Andrew Wilkins
<andrew.wilkins at canonical.com> wrote:
> Hi all,
>
> Anastasia raised an issue in http://reviews.vapour.ws/r/885/ about how to
> cut down on struct conversions between params, state, and domain packages.
> In this case we're talking about storage. The following API server facades
> currently participate in storage:
> - client
> - storage
> - provisioner
> - diskmanager (to be renamed, this lists block devices on machines)
> - diskformatter
> - storageworker (to be renamed, this is the dynamic storage provisioner)
>
> Each facade have some overlap in dealing with storage entities, e.g. the
> diskformatter and diskmanager each need to deal with block devices. This
> leads to much duplication of struct copying/conversion code when toing and
> froing between state and clients.
>
> I don't want to go adding conversion code to the params, state or storage
> packages, as they really shouldn't have dependencies on each other. Does
> anyone have a good idea about where to put this common functionality? Maybe
> "api/common/storage", "apiserver/common/storage"? Does not appeal, but I
> can't think of a better option.
>
> Cheers,
> Andrew
>
> --
> Juju-dev mailing list
> Juju-dev at lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/juju-dev
>
More information about the Juju-dev
mailing list