[Freeipa-devel] [PATCH] Don't load the LDAP schema during startup

[Rob Crittenden]

Jan Zelený wrote:
> Jan Zelený<jzeleny at redhat.com>  wrote:
>> Jan Zeleny<jzeleny at redhat.com>  wrote:
>>> Jan Zelený<jzeleny at redhat.com>  wrote:
>>>> Jan Zelený<jzeleny at redhat.com>  wrote:
>>>>> Rob Crittenden<rcritten at redhat.com>  wrote:
>>>>>> Jan Zelený wrote:
>>>>>>> Rob Crittenden<rcritten at redhat.com>   wrote:
>>>>>>>> Jan Zelený wrote:
>>>>>>>>> Loading of the schema is now performed in the first request
>>>>>>>>> that requires it.
>>>>>>>>>
>>>>>>>>> https://fedorahosted.org/freeipa/ticket/583
>>>>>>>>>
>>>>>>>>> Jan
>>>>>>>>
>>>>>>>> We still need to enforce that we get the schema, some low-level
>>>>>>>> functions depend on it. Also, if the UI doesn't get its aciattrs
>>>>>>>> (which are derived from the schema) then nothing will be
>>>>>>>> editable.
>>>>>>>>
>>>>>>>> I'm getting this backtrace if I force no schema by disabling
>>>>
>>>> get_schema:
>>>>>>> Ok, I'm sending new version, it should handle these exceptions
>>>>>>> better and the operation should fail if it needs the schema and
>>>>>>> the schema is not available for some reason.
>>>>>>
>>>>>> This breaks the XML-RPC server. I fixed one problem:
>>>>>> --- a/ipaserver/plugins/ldap2.py
>>>>>> +++ b/ipaserver/plugins/ldap2.py
>>>>>>
>>>>>> @@ -253,9 +253,10 @@ class ldap2(CrudBackend, Encoder):
>>>>>>        def get_syntax(self, attr, value):
>>>>>>            if not self.schema:
>>>>>> -            self.schema = get_schema(self.ldap_uri, self.conn)
>>>>>> -            if not self.schema:
>>>>>> +            schema = get_schema(self.ldap_uri, self.conn)
>>>>>>
>>>>>> +            if not schema:
>>>>>>                    return None
>>>>>>
>>>>>> +            object.__setattr__(self, 'schema', schema)
>>>>>>
>>>>>>            obj = self.schema.get_obj(_ldap.schema.AttributeType,
>>>>>>            attr)
>>>>>>
>>>>>>            if obj is not None:
>>>>>>                return obj.syntax
>>>>>>
>>>>>> But simply things like get_entry() return an InternalError now. I'm
>>>>>> not sure where you were going by adding this.
>>>>>>
>>>>>> rob
>>>>>
>>>>> Ok, no problem. It's possible that I simply did a mistake thinking I
>>>>> can do something in Python what is not really possible.
>>>>>
>>>>> About that InternalError: I think raising InternalError when we
>>>>> cannot load the schema to do the decoding is the right thing to do.
>>>>> Do you have a better solution? I thought about returning empty
>>>>> result, but that would mean we have to check the result in every
>>>>> funtction that is calling them and raising InternalError there.
>>>>
>>>> I'm sending updated patch. I modified the get_syntax() as you suggested
>>>> and I slightly modified raising that InternalError - currently it isn't
>>>> raised when results from get_entry() are not required by calling
>>>> method. Currently I'm running some tests, preliminary results looked
>>>> ok.
>>>
>>> self-nack
>>>
>>> I discovered some issues discovered by internal test suite, I'm working
>>> on them
>>>
>>> Jan
>>
>> Ok, everything is solved, I'm sending final version of the patch in the
>> attachment. But I still think this should go to 2.1, since it's quite
>> extensive patch in the core of IPA server and it has potential to break
>> many things.
>>
>> Jan
>
> Rebased against master
>
> Jan

I came up with an alternate patch that just does lazy retrieval of the 
schema rather than getting it at start up. Without the schema we can do 
very little required enforcement so we shouldn't proceed.

See my patch 787 Don't load LDAP schema at startup

rob