[Freeipa-devel] [PATCH 0029] suppress errors arising from deleting non-existent files during client uninstall

[Martin Babinsky]

On 04/14/2015 04:24 PM, Martin Basti wrote:
> On 14/04/15 16:12, Martin Basti wrote:
>> On 14/04/15 14:25, Martin Babinsky wrote:
>>> This patch addresses https://fedorahosted.org/freeipa/ticket/4966
>>>
>>> The noise during rollback/uninstall is caused mainly by unsuccessful
>>> attempts to remove files that do not exist anymore. These errors are
>>> now logged at debug level and do not pop-up to stdout/stderr.
>>>
>>>
>>>
>> Hello, thank you for the patch.
>>
>> 1)
>> The option add_warning is quite unclear to me. It does not show
>> warning but error. I suggest something like,  show_hint,
>> show_user_action, or something show_additional_..., or
>> promt_manual_removal
>>
>> Martin^2
>>
>>
> Continue...
>
> 2)
>
>              if file_exists(preferences_fname):
>                  try:
>                      os.remove(preferences_fname)
>                  except OSError as e:
>                      log_file_removal_error(e, preferences_fname, True)
>
> In this case file not found error should never happen.
>
> Could you remove the 'if file_exists' part and handle just exception?
>
I just reverted this bit to original form in order to not fix something 
that isn't broken. Is that ok?
> 3)
> this is inconsistent with change above, choose one style please:
>
>              if os.path.exists(ca_file):
>                  try:
>                      os.unlink(ca_file)
>                  except OSError, e:
>                      root_logger.error(
>                          "Failed to remove '%s': %s", ca_file, e)
>
> --
> Martin Basti
>

Attaching updated patch.

-- 
Martin^3 Babinsky
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbabinsk-0029-2-suppress-errors-arising-from-deleting-non-existent-f.patch
Type: text/x-patch
Size: 3511 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150415/19af0635/attachment.bin>