[Freeipa-devel] [PATCH] 814-818 migrate-ds: optimize adding users to default group

[Petr Vobornik]

On 04/30/2015 03:53 PM, Martin Basti wrote:
> On 10/04/15 12:55, Petr Vobornik wrote:
>> The essential patch is 814.
>>
>> 815 a proposal for new option.
>>
>> 816 and 818 are cleanup patches.
>>
>> 817 little optimization.
>>
>> == [PATCH] 814 migrate-ds: optimize adding users to default group ==
>> Migrate-ds searches for user without a group and adds them to default
>> group. There is no point in checking if the user's selected by
>> previous query are not member of default group because they are not
>> member of any group.
>>
>> The operation is also speeded up by not fetching the default group.
>> Users are added right away.
>>
>> https://fedorahosted.org/freeipa/ticket/4950
>>
> NACK
>
> Users haven't been added into ipa default group after migration.

Fixed in patch 815.

>
> Just nitpick
>
> 1) too many parentheses
>              api.log.error(('Adding new members to default group failed:
> %s \n'
>                             'members: %s') % (e, (','.join(member_dns))))
> You can use this instead:
>              api.log.error('Adding new members to default group failed:
> %s \n'
>                            'members: %s', e, ','.join(member_dns))

Fixed.

>
> == [PATCH] 815 migrate-ds: skip default group options ==
>> New option --use-default-group=False could be used to disable adding of
>> migrated users into default group.
>>
>> By default, the default group is no longer POSIX therefore it doesn't
>> fulfill the original idea of providing GID and therefore it could be
>> skipped during migration.
> LGTM

the option got so the default would be applied.
+            autofill=True,


>>
>> == [PATCH] 816 migrate-ds: remove unused def_group_gid context
>> property ==
>> it's no longer used anywhere
>>
> 1)
> You can remove the unused variable 'g_attrs'

removed

>> == [PATCH] 817 migrate-ds: optimize gid checks by utilizing dictionary
>> nature of set ==
>>
> LGTM
>> == [PATCH] 818 migrate-ds: log migrated group members only on debug level ==
>> It pollutes error_log.
>>
> 1)
> you do not need % formatting in logger
> api.log.debug('migrating %s group %s' , member_attr, m)

fixed and also changed "migrating %s user %s'" line to debug, which was 
the actual reason for this patch.

>>
>>
>
> Martin^2
>


-- 
Petr Vobornik
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pvoborni-0818-1-migrate-ds-log-migrated-group-members-only-on-debug-.patch
Type: text/x-patch
Size: 1397 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150505/03f8864f/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pvoborni-0817-1-migrate-ds-optimize-gid-checks-by-utilizing-dictiona.patch
Type: text/x-patch
Size: 1877 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150505/03f8864f/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pvoborni-0816-1-migrate-ds-remove-unused-def_group_gid-context-prope.patch
Type: text/x-patch
Size: 1338 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150505/03f8864f/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pvoborni-0815-1-migrate-ds-skip-default-group-options.patch
Type: text/x-patch
Size: 5146 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150505/03f8864f/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pvoborni-0814-1-migrate-ds-optimize-adding-users-to-default-group.patch
Type: text/x-patch
Size: 2988 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150505/03f8864f/attachment-0004.bin>