On not copy-pasting state and params struct conversions

[Andrew Wilkins]

On Tue, Feb 10, 2015 at 5:44 PM, William Reade <william.reade at canonical.com>
wrote:

> 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).
>

Not sure I follow what you're saying about "in terms of the storage types",
so
I'll describe how it is at the moment.

The types are in three places:
  - juju/juju/storage
  - juju/juju/apiserver/params
  - juju/juju/state

There's a storage.Volume, a params.Volume, and a state.Volume. state.Volume
is actually an interface, but there's a VolumeInfo which ~correlates to the
other
struct types.

I think the conversion from params<->state all needs to be in
apiserver/<something>.

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?
>

I could live with it, but I don't like the idea of adding dependencies to
every domain
for every facade that uses it.

Cheers,
Andrew

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
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/juju-dev/attachments/20150210/5a13ab83/attachment.html>