On 01.08.2011 16:13, Adam Young wrote:
> On 08/01/2011 06:34 AM, Alexander Bokovoy wrote:
>> Hi,
>> while investigating #1549 and #1550 I stumbled upon a problem. We create
>> Param(s) as read only entities. This means that using standard methods,
>> any modifications to Param instances are denied. What happens in #1549
>> and #1550 is that the code in Param.validate() relies on availability of
>> existing context to determine whether name or cli_name of a param
>> instance should be used.
>> By itself it is fine as context is passed to Param.validate() within
>> environment. The decision what to pass is done in Param.__call__():
>>      def __call__(self, value, **kw):
>>          """
>>          One stop shopping.
>>          """
>>          if value in NULLS:
>>              value = self.get_default(**kw)
>>          else:
>>              value = self.convert(self.normalize(value))
>>          if hasattr(self, 'env'):
>>              self.validate(value, self.env.context)
>>          else:
>>              self.validate(value)
>>          return value
>> If this Param instance has attribute 'env', we use its context. However,
>> the instance in itself is ReadOnly and gets locked at the very end of
>> Param.__init__().
>> So this makes a case when immediately after creating a parameter we
>> can't assign it any environment, its environment always stays None and
>> the code in __call__() always calls validate() without context.
>> A fix I found is quite bad as it violates ReadOnly promise:
>> diff --git a/ipalib/frontend.py b/ipalib/frontend.py
>> index 3534310..1c7071a 100644
>> --- a/ipalib/frontend.py
>> +++ b/ipalib/frontend.py
>> @@ -344,6 +344,9 @@ class HasParam(Plugin):
>>           for spec in get():
>>               param = create_param(spec)
>>               if env is None or param.use_in_context(env):
>> +                if env is not None and not hasattr(param, 'env'):
>> +                    # Force specified environment
>> +                    object.__setattr__(param, 'env', env)
>>                   yield param
>>       def _create_param_namespace(self, name, env=None):
>> Does anybody have better suggestion?
> What is the env supposed to represent?  Does it pre-exist the Param
> creation?  If so, it should be passed to the Param constructor.
At least in CLI case it does pre-exist:
 (options, argv) = api.bootstrap_with_global_options(context='cli')
 for klass in cli_plugins:

context (and env with it) is created before any plugin is loaded (where
Params are created aftewards, when plugins are loaded and initialized).

But environment is not available to Params when they created unless you
would be referencing api.env directly...

We use same trick with object.__setattr__() in api.load_plugins() -- API
instance sets 'plugins' attribute on itself after being locked earlier
in constructor in non-production mode.

> OTOH, if the env is something that can change, then we should not
> require it for the validate call.  Is it possible to validate without an
> env?
validate uses it to issue RequirementError exception with correct param
name according to the context -- for CLI it wants to use cli_name. As
context is always None we get what is shown in #1549 and #1550.

/ Alexander Bokovoy

Freeipa-devel mailing list

Reply via email to