On Tue, Feb 10, 2015 at 6:16 PM, roger peppe <[email protected]> wrote:
> On 10 February 2015 at 10:06, Andrew Wilkins > <[email protected]> wrote: > > On Tue, Feb 10, 2015 at 5:57 PM, roger peppe <[email protected]> > > wrote: > >> > >> On 10 February 2015 at 08:55, Andrew Wilkins > >> <[email protected]> wrote: > >> > Hi all, > >> > > >> > Ian raised a good point in http://reviews.vapour.ws/r/885/ (caution > >> > advised, > >> > may cause eyebleed) about a change I made to errors: > >> > https://github.com/juju/errors/pull/17 > >> > > >> > The NotAssigned error, which previously was only raised by > >> > state.Unit.AssignedMachineId, is now also raised by > >> > state.Volume.StorageInstance. I removed the error from the state > package > >> > so > >> > we didn't have apiserver/common poking its head in there, and moved it > >> > over > >> > to the juju/errors package. > >> > > >> > The issue Ian raised is that juju/errors should contain relatively > >> > generic > >> > error types, and we should keep domain-specific things elsewhere. > >> > Examples > >> > are NotAssigned, and NotProvisioned. We're thinking of moving these > >> > domain-specific error types into a new package, juju/juju/errors. What > >> > do > >> > you think about this? > >> > >> I agree with Ian here. Personally, I think that specific error > >> types/values work well when defined in or close to the module that > returns > >> them, > >> which is aligned to my general feeling that the set of possible returned > >> errors is part of the contract of the method it was returned from. > >> > >> For example, the mgo package returns mgo.ErrNotFound when > >> an item isn't found, and that's fine - we translate from that > >> error to a more local not-found error when necessary. > >> > >> For errors that pass over an API, they can be considered a part of the > API > >> and defined along with the other API types. > >> > >> So, rather than a single package for juju-related errors, > >> I would suggest that domain-specific errors are defined > >> in packages close to the domain in question rather than > >> in a single package used by everything. To my mind this > >> is a more modular and scalable approach. > > > > > > That all sounds good, but it doesn't work very well with the existing > > error code thingy in apiserver/common, unless we're okay with > > apiserver/common poking into state. I'm not a fan of that. > > > > Perhaps we just need a state-internal package which registers predicates > > in apiserver/common to do that magic. That wouldn't be so bad IMO. > > I don't understand. Can't you just define NotAssignedError in state > and have the usual predicate operations in apiserver/common ? > (you can't import apiserver/common from state BTW - you'd get a cycle) > Yeah, I didn't think that through well. Long day. Having it state dependency from apiserver/common is, as William points out, fine. It's all going to state anyway.
-- Juju-dev mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
