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:

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
+                    profile_id))
+            else:
+                root_logger.error("Error migrating profile '{}':
+                    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

Manage your subscription for the Freeipa-devel mailing list:
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to