[PATCH 1 of 2] model: check for invalid users centrally
Thomas De Schampheleire
patrickdepinguin at gmail.com
Tue Jun 23 13:43:14 EDT 2015
On Tue, Jun 23, 2015 at 3:59 PM, Mads Kiilerich <mads at kiilerich.com> wrote:
> On 06/22/2015 10:09 PM, Thomas De Schampheleire wrote:
>> On Mon, Jun 22, 2015 at 7:05 PM, Mads Kiilerich <mads at kiilerich.com>
>>> On 06/22/2015 06:45 PM, Thomas De Schampheleire wrote:
>>>> # HG changeset patch
>>>> # User Thomas De Schampheleire <thomas.de.schampheleire at gmail.com>
>>>> # Date 1434984724 -7200
>>>> # Mon Jun 22 16:52:04 2015 +0200
>>>> # Node ID 68032f8bd7f52c676bb5143a03b0dfd5df242f0c
>>>> # Parent 7303d0035d82a20f93df1c78b3358403b7ff9827
>>>> model: check for invalid users centrally
>>>> Commit 9a23b444a7fefe5b67e57a42d26343787d992874 introduced
>>>> UserInvalidException. The raising of this exception, however, was done
>>>> in pullrequest context, while it could have been done in
>>> Why not be more consistent and do it in _get_instance (using a more
>>> EntityNotFound exception (or something like that)? Is that the end goal
>>> this just an intermediate step?
>> Would you translate the EntityNotFound in some other more specific
>> exception class, for example from _get_user, or would EntityNotFound
>> be the final exception?
> I think that would be fine.
> In some cases, there will be multiple users involved anyway and it will not
> be possible to distinguish _which_ of them failed. The description should of
> course explain what it was that failed.
>> This user part is an evolution from my initial introduction of
>> UserInvalidException, but as I hinted below I think it makes sense to
>> expand it. Doing it directly in get_instance is indeed more logical, I
>> can do that at the same time.
> Ok - so you will rework this patch to do that?
Yes, I will...
More information about the kallithea-general