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
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
