[Freeipa-devel] [PATCH] 0009 Use cli_name if possible to return errors via exceptions in Param

Alexander Bokovoy abokovoy at redhat.com
Tue Aug 2 14:51:49 UTC 2011


On 02.08.2011 15:27, Alexander Bokovoy wrote:
> Following yesterday's discussion on IRC with Rob, I further investigated
> the issue and came up with a following fix (attached).
> 
> The patch extends arguments supported by Param class to accept
> environment and set it if it is not None before locking down the class.
> 
> It further extends create_param() helper function to re-create params
> from a passed spec if provided environment is not None.
> 
> Command and Object now use create_param() to inject their environment
> into specified arguments, parameters, and options.
> 
> There are might be more cases where using create_param(spec,
> env=self.env) would be needed as I have found that, for example, running
> 
> ipa user-add
> 
> and not passing first name (--first) would give you
> 
> $ ./ipa user-add
> First name:
> ipa: ERROR: 'givenname' is required
> 
> while actual error should be
> 
> ipa: ERROR: 'first' is required
> 
> This is solved by the attached patch by introducing create_param() call
> into Object while fixes in Command will give you protection for
> client-side commands as well.
> 
> If approach is fine, I can find and fix other places.
Self NACK. The problem is that it is pushing env into API and that
complicates situation dramatically as unless you ignore Env in _repr()
you are getting different API.txt every time makeapi script is run even
though there are no really changes in API.

I'm going to look deeper into how to avoid that in 2.1.1 but it looks
like original two-line patch which injects Env violating ReadOnly
promises is working (unit tests pass) and less invasive. Sort of
neccessary evil...
-- 
/ Alexander Bokovoy




More information about the Freeipa-devel mailing list