-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 10.02.2015 11:44, William Reade 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). +1
I've added TODO comments to fields and structs in params and state where we are using field types from external packages, like network. We need to be *very* careful not to do that, because both params request/response types and state docs should serialize the same way and not change unexpectedly if the external package's type changes. Otherwise we'll end up with hard to debug issues and incompatibilities between juju versions. > > 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? +1 Most of these conversions are straightforward and increase the amount of boilerplate code. Using code generation scripts can help to make the initial step of "making a params.Foo/state.Foo out of somepackage.Foo" easier, but after that we need to check the code and maintain it as-is. Please, always keep in mind the need for stability of the serialized representations for things that go on the wire (API request/response types and their fields) or in Mongo (doc types and their fields). > > 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). We can think about having tool to check we're not violating the API/state type stability after we refactor the existing cases. > > 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. I think as a rule of thumb, "to each its own" is useful here - e.g. storage types need to be in storage package (or a subpackage thereof to allow importing without loops), but things that go in state or over the wire via the API need to be in subpackages of state and params. >> >> Cheers, Andrew >> >> -- Juju-dev mailing list [email protected] Modify >> settings or unsubscribe at: >> https://lists.ubuntu.com/mailman/listinfo/juju-dev >> > - -- Dimiter Naydenov <[email protected]> Juju Core Sapphire team <http://juju.ubuntu.com> -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJU2d3ZAAoJENzxV2TbLzHwN1gIAK9eaSydcIhlmXGw1zue+mq1 86MeQjmQwB9l7C/Mqv/gK6MXfI7JUDB9cfgQ80ltyRLWhOI0rO2h/ICISDj3bEgp F5SV7iZ8m9CdFhAuiS+waotzomojOiO1IZXQv5FCGCQ0yVAatYIezL/hH3jfRXS9 bRKJZDaaEYa+ORkPhu1rkAV9CDjbg4jS+T8tbkvZYcVkQjxRAPhITrMXL8uYzLep Zzk8yU8+8tbq5lXbg99mNdQa0MSeU3gj6X1PzlTPycSTCxrr6Oviq1YaWD4lvfYQ Mln27yW3hFbd8HAK6m7q4v5JS57auZO9H69f9rFvm33+TazUjShz5gOpZbQDf0A= =+TQK -----END PGP SIGNATURE----- -- Juju-dev mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
