[Freeipa-devel] [PATCHES] 0014, 0016 [RFE] ipa migrate-ds should have an argument to specify cert to use for DS connection

[Petr Viktorin]

On 10/21/2013 10:29 AM, Martin Basti wrote:
> On Mon, 2013-10-21 at 09:29 +0200, Martin Kosek wrote:
>> On 10/18/2013 05:00 PM, Martin Basti wrote:
>> > Patch attached.
>> >
>> > Ticket:
>> >https://fedorahosted.org/freeipa/ticket/3243
>> >
>>
>> I did not test the patch, just looked at the code and I have few comments:
>>
>> 1) Please put the ipalib/cli.py change to a sepparate change, we may want to
>> backport it separately some day and this will make it easier.
>>
> Separated.
> Patch 0014-2 -- CLI

Nitpick: if you switched them around it would be easier to read*:
     if raw:
         decode
     elif required:
         raise
*(at least for me, I need way too much concentration to process boolean 
logic)


> Patch 0016 -- migration

We have a utility, ipautil.write_tmp_file, that should do what you need 
here.
With this the created file is removed some time later when there are no 
more references to the file object, so no need for the try: block.
(btw, if the try block was necessary, it should have also covered the 
write() call.)

Also, since you changed the API, please run ./makeapi and bump the API 
version in the VERSION file.

-- 
Petr³