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

[Jan Cholasta]

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().

-- 
Jan Cholasta