[Freeipa-devel] [PATCH 0065] Use private ccache in ipa-server-install

[Petr Viktorin]

On 06/04/2013 01:29 PM, Tomas Babej wrote:
> On 06/03/2013 02:58 PM, Martin Kosek wrote:
>> On 06/03/2013 02:43 PM, Tomas Babej wrote:
>>> Hi,
>>>
>>> this patch fixes the installation problems on master on F19 with krb5
>>> packages
>>>> = 1.11.2-6
>>> https://fedorahosted.org/freeipa/ticket/3666
>>>
>>> Tomas
>> 1) Leaving cache_desc open:
>>
>> +        (cache_desc, cache_path) = tempfile.mkstemp(prefix='krbcc')
>> +        os.environ['KRB5CCNAME'] = cache_path
>>
>> Why do we keep the descriptor open and close it at the and of the
>> installation?
>> Can we close it right after tempfile.mkstemp? I think we do it this
>> way in
>> other places in installation.
>>
>> 2) What about other installers where we handle Kerberos auth, like
>> ipa-{replica,dns,ca}-install?
>>
>> A common function, other shared means, of handling KRB5CCNAME may be
>> appropriate to avoid duplicating code too much.
>>
>> Martin
> I moved the code responsible to PrivateCCache class, both for
> readability and conciseness.
>
> Private ccache now used in replica,dns and ca the installers. I managed
> to reproduce the error only with
> dns-install though(fails on adding the service principal), but having a
> private ccache for the installer should not hurt.
>
> Ipa-adtrust-install requires the admin ticket, so there shouldn't be an
> issue.
>
> Tomas

Shouldn't the context manager restore os.environ['KRB5CCNAME'] on exit?


Two nitpicks:

ipaserver/install/installutils.py: new blank line at EOF

For most context managers I prefer @contextlib.contextmanager rather 
than writing out the class, it makes them shorter and easier to read.


-- 
Petr³