On Tue, Feb 10, 2015 at 6:13 PM, William Reade <[email protected]> wrote:
> I think that NotAssigned is *too* generic. UnitNotAssignedToMachineErr > is one thing, and VolumeNotAssignedToStorageInstance is another; if we > Okay, fair point; I saw "IsCodeNotAssigned" in apiserver/common and got a bit carried away. That really should be "IsCodeUnitNotAssigned" then. I'll put the error back in state and add a new, volume-specific one. > 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
