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.

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))

== [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

== [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'
== [PATCH] migrate-ds: optimize gid checks by utilizing dictionary nature of set ==

LGTM
== [PATCH] 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)



Martin^2

--
Martin Basti

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to