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

[Jan Cholasta]

On 23.2.2016 09:55, Martin Babinsky wrote:
> 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).

Thanks, ACK.

Pushed to:
master: 2c3b0b1bcd972e6beec4691c03830f37dd27e199
ipa-4-2: 704319c3eaf74e0531dd2aa1e5880db7b6ab830c
ipa-4-3: e0ce7e37636969af56a4fa2b1068ae97f1058bdd

-- 
Jan Cholasta