On Fri, 2016-07-29 at 15:19 +0200, Martin Basti wrote: > > 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
I considered this, but I rarely found that imports were such a big issue, usually it's code in the file that is, so IMO the trade-off is worth it. > 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. Sure backports will be somewhat harder, but I wouldn't say it is a nightmare, it is not code logic that changes, it is just individual import lines. > However plus is, when we once fix it, we can enable pylint check to keep > it right forever. Exactly, this is the appealing point, we get it right once and then the tool keeps us honest going forward. > We can open refactoring ticket and we'll see. Please do. Simo. -- Simo Sorce * Red Hat, Inc * New York -- 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
