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

[Martin Basti]

On 16.08.2016 12:06, Stanislav Laznicka wrote:
> 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.
>

You use RuntimeError in bindinstance.py, in other files you use 
ScriptError, is this RuntimeError on purpose there?

Martin^2