[Freeipa-devel] [PATCH] 12 When migrating warn user if compat is enabled

[Rob Crittenden]

Ondrej Hamada wrote:
> On 02/29/2012 05:07 PM, Rob Crittenden wrote:
>> Ondrej Hamada wrote:
>>> On 02/28/2012 10:52 PM, Rob Crittenden wrote:
>>>> Ondrej Hamada wrote:
>>>>> On 02/27/2012 09:47 PM, Rob Crittenden wrote:
>>>>>> Ondrej Hamada wrote:
>>>>>>> On 02/21/2012 02:32 PM, Ondrej Hamada wrote:
>>>>>>>> On 02/20/2012 06:53 PM, Rob Crittenden wrote:
>>>>>>>>> Ondrej Hamada wrote:
>>>>>>>>>> https://fedorahosted.org/freeipa/ticket/2274
>>>>>>>>>>
>>>>>>>>>> Added check into migration plugin to warn user when compat is
>>>>>>>>>> enabled.
>>>>>>>>>> If compat is enabled, the migration fails and user is warned
>>>>>>>>>> that he
>>>>>>>>>> must turn the compat off or run the script with (the newly
>>>>>>>>>> introduced)
>>>>>>>>>> option '--compat'.
>>>>>>>>>>
>>>>>>>>>> '--compat' is just a flag, by default set to false. If it is
>>>>>>>>>> set, the
>>>>>>>>>> compat check is skipped.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Interesting approach. I think this is probably good, preventing
>>>>>>>>> migration when the compat plugin is enabled unless you
>>>>>>>>> specifically
>>>>>>>>> decide to.
>>>>>>>>>
>>>>>>>>> I think the option may need another name, maybe --with-compat or
>>>>>>>>> something.
>>>>>>>>>
>>>>>>>>> I think in the message we should use "enabled" instead of "on".
>>>>>>>>> That
>>>>>>>>> is the language of ipa-compat-manage.
>>>>>>>>>
>>>>>>>>> The migration help should have a discussion of why this is a
>>>>>>>>> problem
>>>>>>>>> too, and what compat really is (provides a different view of the
>>>>>>>>> data
>>>>>>>>> to be compatible with non RFC2703bis systems).
>>>>>>>>>
>>>>>>>>> rob
>>>>>>>> corrected
>>>>>>>>
>>>>>>>> Ondra
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> Freeipa-devel mailing list
>>>>>>>> Freeipa-devel at redhat.com
>>>>>>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>>>>>> I forget to update the commit message about the change of flag name.
>>>>>>> Corrected patch attached.
>>>>>>>
>>>>>>
>>>>>> This works ok it just seems to be making an assumption on the client
>>>>>> when to print this. I think a similar value like enabled needs to be
>>>>>> created to explicitly say why we are returning.
>>>>>>
>>>>>> rob
>>>>> sorry for that, value created
>>>>>
>>>>> Ondra
>>>>>
>>>>
>>>> I think you need to define beter what compat means in the output, it
>>>> coudl be very confusing. You can return a value for it without testing
>>>> whether it is actually a problem or not.
>>>>
>>>> I think what compat is supposed to mean is "Am I failing because of
>>>> compat" and not an indication of whether compat is enabled or not.
>>>>
>>>> Some documentation at a minimum should be added.
>>>>
>>>> It otherwise seems to work ok.
>>>>
>>>> rob
>>>> You could return a value for compat here without
>>> I've updated the description of 'compat' value in output and also
>>> changed the condition when this value is set to False. Now it is set to
>>> False only when the migration fails because of compatibility plugin.
>>>
>>
>> Code looks good. I think the error language needs some tweaking.
>>
>> I think the help text should read:
>>
>> The schema compat feature allows IPA to reformat data for systems that
>> do not support RFC2307bis. It is recommended that this be disabled
>> during migration to reduce system overhead. It can be re-enabled after
>> migration. To migrate with it enabled use the "--with-compat" option.
>>
>> I think the client-side error should read:
>>
>> The compat plug-in is enabled. This can increase the memory
>> requirements during migration. Disable the compat plug-in with
>> \'ipa-compat-manage disable\' or re-run this script with
>> \'--with-compat\' option."
>>
>> rob
> patch attached
>
> Ondra
>

ACK, pushed to master and ipa-2-2