A home for underappreciated errors

[roger peppe]

On 10 February 2015 at 10:06, Andrew Wilkins
<andrew.wilkins at canonical.com> wrote:
> On Tue, Feb 10, 2015 at 5:57 PM, roger peppe <roger.peppe at canonical.com>
> wrote:
>>
>> On 10 February 2015 at 08:55, Andrew Wilkins
>> <andrew.wilkins at canonical.com> 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)