[Freeipa-devel] [PATCH] 0130 -- create missing idranges in trust-fetch-domains

[Martin Kosek]

On 01/14/2014 01:02 PM, Alexander Bokovoy wrote:
> Hi,
> 
> attached patch implements missing idranges when new child domains
> discovered through 'ipa trust-fetch-domains'. This functionality existed
> in 'ipa trust-add' but was not exposed in the 'ipa trust-fetch-domains'.
> 
> Additionally, error message is shown in case trust name is incorrect.
> 
> https://fedorahosted.org/freeipa/ticket/4104
> https://fedorahosted.org/freeipa/ticket/4111
> 

I did not test the patch, just few observations from reading it:

1) Why are you moving add_range method from trust object to the module? IMO it
could be left there, it belongs to the object. Plus, the patch won't be that
big and easier to backport. add_range can be still referred from other commands
as "self.obj.add_range", no need to move it.

2) This part looks *very* suspicious:

-        trust = self.api.Command.trust_show(keys[0], raw=True)['result']
+        try:
+            trust = self.api.Command.trust_show(keys[0], raw=True)['result']
+        except AssertionError:

I do not think that AssertionError should be raised and caught in normal
operation, it should be an exceptional exception raised when the world falls
apart IMO. I.e. I would rather see some PublicError or Exception based
exception to be raised in trust_show in that case...

Martin