[Freeipa-devel] [PATCH 0052] Only specify the ipatokenuniqueid default in the add operation

[Alexander Bokovoy]

On Thu, 22 May 2014, Petr Viktorin wrote:
>On 05/22/2014 05:13 PM, Petr Vobornik wrote:
>>On 22.5.2014 17:00, Nathaniel McCallum wrote:
>>>On Thu, 2014-05-22 at 10:53 -0400, Nathaniel McCallum wrote:
>>>>On Thu, 2014-05-22 at 16:45 +0200, Petr Viktorin wrote:
>>>>>On 05/22/2014 04:12 PM, Nathaniel McCallum wrote:
>>>>>>On Tue, 2014-05-13 at 12:55 -0400, Nathaniel McCallum wrote:
>>>>>>>On Tue, 2014-05-13 at 16:47 +0200, Jan Cholasta wrote:
>>>>>>>>On 12.5.2014 20:50, Nathaniel McCallum wrote:
>>>>>>>>>On Mon, 2014-05-12 at 18:40 +0200, Misnyovszki Adam wrote:
>>>>>>>>>>On Tue, 06 May 2014 11:46:14 -0400
>>>>>>>>>>Nathaniel McCallum <npmccallum at redhat.com> wrote:
>>>>>>>>>>
>>>>>>>>>>>On Tue, 2014-05-06 at 11:38 -0400, Nathaniel McCallum wrote:
>>>>>>>>>>>>On Tue, 2014-05-06 at 17:34 +0200, Petr Vobornik wrote:
>>>>>>>>>>>>>On 6.5.2014 17:13, Nathaniel McCallum wrote:
>>>>>>>>>>>>>>On Tue, 2014-05-06 at 17:04 +0200, Petr Vobornik wrote:
>>>>>>>>>>>>>>>On 6.5.2014 16:51, Nathaniel McCallum wrote:
>>>>>>>>>>>>>>>>Specifying the default in the LDAP Object causes the
>>>>>>>>>>>>>>>>parameter to be specified for non-add operations. This is
>>>>>>>>>>>>>>>>especially problematic when performing the modify operation
>>>>>>>>>>>>>>>>as it causes the primary key to change for every
>>>>>>>>>>>>>>>>modification.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>https://fedorahosted.org/freeipa/ticket/4227
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>shouldn't removal of `autofill=True,` be enough?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>Removing autofill=True results in the default not being used
>>>>>>>>>>>>>>for the otptoken-add operation. That may be a different bug
>>>>>>>>>>>>>>(I'm not sure what the expectation of autofill is).
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>Nathaniel
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>Seems to work form me with:
>>>>>>>>>>>>>
>>>>>>>>>>>>>diff --git a/ipalib/plugins/otptoken.py
>>>>>>>>>>>>>b/ipalib/plugins/otptoken.py index f68ea7d..623f1f1 100644
>>>>>>>>>>>>>--- a/ipalib/plugins/otptoken.py
>>>>>>>>>>>>>+++ b/ipalib/plugins/otptoken.py
>>>>>>>>>>>>>@@ -121,9 +121,7 @@ class otptoken(LDAPObject):
>>>>>>>>>>>>>                 cli_name='id',
>>>>>>>>>>>>>                 label=_('Unique ID'),
>>>>>>>>>>>>>                 default_from=lambda: unicode(uuid.uuid4()),
>>>>>>>>>>>>>-            autofill=True,
>>>>>>>>>>>>>                 primary_key=True,
>>>>>>>>>>>>>-            flags=('optional_create'),
>>>>>>>>>>>>>             ),
>>>>>>>>>>>>>             StrEnum('type?',
>>>>>>>>>>>>>                 label=_('Type'),
>>>>>>>>>>>>
>>>>>>>>>>>>Doing this causes the ipa otptoken-add command to prompt for the
>>>>>>>>>>>>Unique ID. This may be the desired behavior, but it is not
>>>>>>>>>>>>how it
>>>>>>>>>>>>worked previously (no prompt).
>>>>>>>>>>>
>>>>>>>>>>>Here is an alternate patch for this second approach. I have no
>>>>>>>>>>>strong
>>>>>>>>>>>opinion on the correct behavior here.
>>>>>>>>>>>
>>>>>>>>>>>Nathaniel
>>>>>>>>>>
>>>>>>>>>>IMO you should update API.txt with ./makeapi
>>>>>>>>>
>>>>>>>>>Running ./makeapi results in no changes to API.txt.
>>>>>>>>
>>>>>>>>This is not right, there *are* changes in the API and build fails
>>>>>>>>for me
>>>>>>>>becase API.txt is not updated.
>>>>>>>
>>>>>>>I think maybe I ran it from the wrong branch. Fixed.
>>>>>>
>>>>>>I still need a review of this. It is pretty trivial.
>>>>>>
>>>>>>Nathaniel
>>>>>
>>>>>This still prompts for the unique ID on add:
>>>>>
>>>>>$ ipa otptoken-add
>>>>>Unique ID [25cb3aa9-db19-40f8-acf4-33ef65ca863c]:
>>>>>
>>>>>I don't think that's the intended behavior.
>>>>
>>>>Hence the alternate patch (0052a). If we don't want to prompt, we'll
>>>>need to use the first patch (0052). I have no strong opinion on the
>>>>correct behavior and I am fine with merging either patch.
>>>
>>>Attached is the non-alternate (0052) with the api updated.
>>>
>>>Nathaniel
>>
>>IMO 52a is better if used by hand and it keeps code cleaner.
>
>I don't think that should influence the design of the CLI that much.
>
>>It might
>>not be ideal though if used from a script because of nonexistent
>>--unattended/-u option which would disable prompt (set
>>env.interactive=False ?).
>
>Right.
>
>
>ACK to the non-prompting version (0052.1).

Pushed to master
* 3f26ff8c37ef3f8a484220f8e514bb2691905d47 Only specify the ipatokenuniqueid default in the add operation

-- 
/ Alexander Bokovoy