[Freeipa-devel] [PATCH 0135] upgrade: unconditional import of certificate profiles into LDAP

[Martin Babinsky]

On 02/23/2016 07:43 AM, Fraser Tweedale wrote:
> On Tue, Feb 23, 2016 at 07:32:31AM +0100, Jan Cholasta wrote:
>> On 23.2.2016 06:40, Fraser Tweedale wrote:
>>> On Mon, Feb 22, 2016 at 02:03:49PM +0100, Martin Babinsky wrote:
>>>> https://fedorahosted.org/freeipa/ticket/5682
>>>>
>>>> --
>>>> Martin^3 Babinsky
>>>>
>>> Thanks for the patch.  Conditional ACK.
>>>
>>> Patch is tested and works, but I am wary about checking for
>>> substring match against RemoteRetrieveError reason string (see hunk
>>> below).  It would be better to carry the status code as an attribute
>>> of RemoteRetrieveError and check whether it is 409.
>>>
>>>> +        except errors.RemoteRetrieveError as e:
>>>> +            if "Profile already exists" in e.reason:
>>>> +                root_logger.debug("Profile '{}' already present".format(
>>>> +                    profile_id))
>>>> +            else:
>>>> +                root_logger.error("Error migrating profile '{}': {}".format(
>>>> +                    profile_id, e))
>>>> +                return
>>>
>>> If you agree, we can file a ticket and I am happy for these patches
>>> to be merged as-is.  The scope of changing RemoteRetrieveError is
>>> larger than #5682 so it makes sense to do it separately, and just
>>> for master branch.
>>
>> I don't think this is the right approach. create_profile() should raise
>> DuplicateEntry rather than RemoteRetrieveError if the profile already
>> exists, which can then be properly handled in _create_dogtag_profile().
>>
> I am happy with that.
>

Attaching updated patch. Unfortunately, REST API does report only a 
super-generic error code 400: Bad request when adding profile which 
already exists, which is not very useful for us (I will open a ticket 
for Dogtag for this).

After consultation with Jan, we decided to log the error at debug level 
and be done with it (for the moment).

-- 
Martin^3 Babinsky
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-4-2-mbabinsk-0135.1-upgrade-unconditional-import-of-certificate-profiles.patch
Type: text/x-patch
Size: 2697 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160223/d5cf7c0f/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbabinsk-0135.1-upgrade-unconditional-import-of-certificate-profiles.patch
Type: text/x-patch
Size: 2630 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160223/d5cf7c0f/attachment-0001.bin>