Hey Sebastian,

Quoting Sebastian Spaeth (2011-09-29 09:45:38)
>On Mon, 26 Sep 2011 03:05:29 +0200, Justus Winter wrote:
>
>#1) APPLIED
>#2) APPLIED
>#4) APPLIED
>#5&#6) APPLIED
>#9) APPLIED
>Thanks for the patches, most of them are quite nice. For 7&8, I'd like
>to hear more opinions.

Thanks :)

>#3) reorder the arguments of NotmuchError.__init__(): NOT APPLIED
>
>The python tutorial gives an example of custom TransitionError with
>three arguments, a custom message as the third. In addition, a STATUS
>value is always expected to be given, while the additional explanatory
>msg is optional, so STATUS makes for a more logical 1st parameter IMHO.
>Even if it were the case, it makes for lots of code churn, longer code
>(status=foo) to all Exceptions, and existing third party code would be
>broken. Overall, I think there is more potential for trouble than
>cleanup.

Well, #3 wasn't meant to be used standalone, I just wanted an intermediate
step in the refactoring process.

To address your concern wrt status being optional: I do consider code
doing 'raise NotmuchError(status=...)' to be wrong, one should raise
a more specific error in the first case.

>#7) NOT APPLIED, INPUT SOUGHT :)
>I do see the value of more fine grained exceptions, but I am not sure,
>we need this level of fine-grainedness. It would also make things more
>tricky (the API is still actively evolving, and e.g. 4 days ago, a new
>error status was added), so users of the bindings would now have
> +    NotmuchError,
> +    OutOfMemoryError,
> +    ReadOnlyDatabaseError,
> +    XapianError,
> +    FileError,
> +    FileNotEmailError,
> +    DuplicateMessageIdError,
> +    NullPointerError,
> +    TagTooLongError,
> +    UnbalancedFreezeThawError,
> +    UnbalancedAtomicError,
> +    NotInitializedError
>
> [...]
>
>to check where e.g. Xapian could also hide an Out of Memory.
Well, if the underlying API changes, the bindings will have to be
updated as well. What's wrong with that?

> Do people
>really want to catch, say UnbalancedAtomic errors specifically, rather
>than testing whether an operation succeeded, and check the status code
>if not? I could see the case for NotInitializedError, as that is a bit
>specific to the python bindings and users might want to catch it separately.

Well, those not interested in the exact nature of the failure can
still except NotmuchError as before. I also added the status code to
each class so that legacy style exception handling won't break.

(On second thought, we could also provide a .args property that issues
a deprecation warning when used to make the transition  even smoother.)

>Also, not all "status" are an error, e.g. DuplicateMessageId denotes
>success rather than failure, it just communicates a status.
Hm, I didn't knew that. This feels somewhat strange though.

If I were to design an API that uses an integer to indicate both
success and error, I'd reserve one bit indicating error so testing
for errors can be easily done by anding with a bitmask.

Cheers,
Justus