[Freeipa-devel] [PATCH 0048] Remove sys.exit() from installer modules

[Stanislav Laznicka]

On 08/16/2016 08:36 AM, Stanislav Laznicka wrote:
> On 08/02/2016 01:08 PM, Stanislav Laznicka wrote:
>> On 07/28/2016 10:57 AM, Martin Basti wrote:
>>> Hello,
>>>
>>> suprisingly, patch needs rebase :)
>>>
>>> 1)
>>> Is the script error the right Exception?
>> I chose ScriptError because it's able to change the return value of 
>> the script, which is necessary sometimes. RuntimeError, which may 
>> seem more suitable, would not be able to do that. However, I am open 
>> for ideas on which exception type to use.
>>>
>>> 2)
>>> Can you use rather raise Exception(), instead of raise Exception
>>>
>> Sure, please see the modified attached patch.
>>> 3)
>>> I really hate to print errors to STDOUT from modules and then just 
>>> call exit(1) (duplicated evil), could you replace print('xxx') with 
>>> raise AnException('xxx')
>> Did that, only kept those prints printing directly to stderr. Not 
>> sure if those should be changed as well.
>>
> Bumping for review/opinions.
>
Also a self-NACK, there were some sys imports that should not have been 
there anymore (thanks, mbasti). Attaching a fixed version.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-slaznick-0048-4-Remove-sys.exit-from-install-modules-and-scripts.patch
Type: text/x-patch
Size: 43791 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160816/65bb4b81/attachment.bin>