On Tue, Feb 10, 2015 at 5:44 PM, William Reade <[email protected]> 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 > <[email protected]> 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 > > [email protected] > > Modify settings or unsubscribe at: > > https://lists.ubuntu.com/mailman/listinfo/juju-dev > > >
-- Juju-dev mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
