[Freeipa-devel] [PATCH 0029] suppress errors arising from deleting non-existent files during client uninstall
Martin Basti
mbasti at redhat.com
Fri Apr 17 10:04:35 UTC 2015
On 15/04/15 15:53, Martin Babinsky wrote:
> 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.
>
thanks,
just one nitpick, can you move the new function into installutils, it
can be used in different scripts not just in ipaclient.
--
Martin Basti
More information about the Freeipa-devel
mailing list