I think that NotAssigned is *too* generic. UnitNotAssignedToMachineErr is one thing, and VolumeNotAssignedToStorageInstance is another; if we use the same error type to represent those distinct situations, we really ought to include a bunch more mechanism for finding out *precisely* what should have been assigned to what (because refactoring happens, and methods start returning the same generic error for new reasons, potentially breaking client code that makes assumptions about what that generic error type implies. See worker/uniter/filter/filter.go and search for ErrTerminateAgent for a particularly hair-raising example...)
I see what you're saying re apiserver/common, but I wouldn't really characterise it as "poking into" state: apiserver is generally implemented in terms of state, so that's a perfectly reasonable dependency IMO. For exactly the reasons stated re generic error types in code, the generic error codes in the api are the scary bits I now wish we'd done differently... Cheers William On Tue, Feb 10, 2015 at 10:55 AM, 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? > > 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
