[Freeipa-devel] [PATCH] 005 Show error in serial association

[Petr Vobornik]

On 08/18/2011 06:25 PM, Endi Sukma Dewata wrote:
> On 8/17/2011 10:38 AM, Petr Vobornik wrote:
>> Ticket #1628 - https://fedorahosted.org/freeipa/ticket/1628
>> Unreported insufficient access error
>>
>> This patch is dependant on
>> freeipa-pvoborni-0004-1-error-dialog-for-batch-command.patch.
>>
>> This may be only a checking if approach of this patch is good.
>>
>> I was not sure if this type of error message (result.failed property) is
>> more general or it only appears in adding members. So I put error
>> handling in serial_associator instead of command. If it would be put in
>> command and success will be transformed to error, it will change the
>> behaviour of executing commands - other commands after error won't be
>> executed. If the approach is good, it could be probably better to change
>> it a little and offer same logic for batch_associator.
>>
>> It should be working for adding users to groups, netgroups, roles and
>> assigning hbac rules (tested as non admin user).
>>
>> Modified association test - data in success handler should not be
>> undefined.
>
> Currently with serial associator if there's a failure the rest of the
> commands will not be executed, so it's an existing problem. You can test
> this by adding a user into multiple groups via UI but remove one of the
> groups via CLI just before adding. The user will not be added into the
> remaining groups. Bulk associator doesn't have this problem because it's
> executed as a single command.
>
> I think eventually we want to convert the serial associator to use batch
> commands (no need to do it now, but you can if you want). Once it's
> converted, all error checking (including result.failed) should be done
> in IPA.command so it can be captured by the batch handler.
>
> I'm not sure about other scenarios that will return result.failed, but I
> wouldn't assume it's limited to associations. So I think it should be
> handled in a more generic way in IPA.command.
>
> One other thing, I think we should avoid using plural class name (i.e.
> IPA.errors) because suppose we have a collection of instances of this
> class the variable name could become confusing (e.g. that.errorss :) ).
> IPA.error_list might be better.
>

Reworked.

'Failed' moved to command. On 'failed' success is transformed to error - 
can be change behaviour of serial associator in some commands 
(previously some commands were executed even after 'failed' of 
previous). It isn't probably big issue because they fail probably from 
the same reason (consequent commands would fail to).

- 'failed' message is extended by related object (so user would know for 
which command in the batch it is related to).

- there is still the problem ('no modifications to be performed') I 
wrote about on Aug 18. I think it should be fixed before commiting this 
path.

-- 
Petr Vobornik
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pvoborni-0005-1-Show-error-in-adding-associations.patch
Type: text/x-patch
Size: 7870 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20110822/34eba03e/attachment.bin>