[Freeipa-devel] [PATCH 0197] re-set canonical principal name on migrated users

[Martin Basti]

On 29.07.2016 15:12, Simo Sorce wrote:
> On Fri, 2016-07-29 at 15:10 +0200, Martin Basti wrote:
>> On 29.07.2016 14:42, Florence Blanc-Renaud wrote:
>>> On 07/28/2016 10:56 AM, Martin Babinsky wrote:
>>>> Fixes https://fedorahosted.org/freeipa/ticket/6101
>>>>
>>>> I have also noticed that the principal aliases are not preserved during
>>>> migration from FreeIPA 4.4.
>>>>
>>>> That, however, requires more powerful runes to transform the realm of
>>>> all values and warrants a separate ticket if we even want to support
>>>> migration of user aliases.
>>>>
>>>>
>>>>
>>> Hi Martin,
>>>
>>> thanks for your patch. From a technical standpoint, it looks good to
>>> me as I tested the following scenarios:
>>>
>>> 1/ without --user-ignore-attribute
>>> - call ipa migrate-ds without specifying any attributes to ignore
>>> The user entries are migrated, and contain a migrated krbprincipalname
>>> and krbcanonicalname.
>>> At this point kinit fails but this is expected as the krb attributes
>>> were not re-generated. Login to the web https://hostname/ipa/ui also
>>> fails as expected.
>>> - login to https://hostname/ipa/migration with the user credentials
>>> - perform kinit => OK
>>> - login to https://hostname/ipa/ui => OK
>>>
>>> 2/ with --user-ignore-attribute={krbPrincipalName,krbextradata,...} as
>>> explained in the Migration page [1]
>>> At this point kinit fails as expected, as well as login to the web
>>> ipa/ui.
>>> - login to https://hostname/ipa/migration with the user credentials
>>> - perform kinit => OK
>>> - login to https://hostname/ipa/ui => OK
>>>
>>>
>>> But the patch produces new pep8 complaints:
>>> ./ipaserver/plugins/migration.py:39:1: E402 module level import not at
>>> top of file
>> This is caused by old code, it should not prevent this patch to be
>> acked. Imports are heavily mixed in code already, it is not possible to
>> keep importing right without fixing old ones.
>> Martin^2
> Can we make a patch to fix all import order issues across the code base
> so we can maintain them properly going forward ?
>
> Simo.
>

Is it worth it?

a) it will makes git blame harder, you will have to go always deeper to 
history with any import
b) it makes backporting of patches harder. We fixed unused imports in 
4.4, and it was PITA to backport patches, always conflicts, several 
bugs. Changing all import to correct order will be even worse.

However plus is, when we once fix it, we can enable pylint check to keep 
it right forever.

We can open refactoring ticket and we'll see.

Martin^2