On 20/04/15 10:58, Martin Babinsky wrote:
On 04/20/2015 10:32 AM, Martin Basti wrote:
On 17/04/15 14:11, Martin Babinsky wrote:
On 04/17/2015 12:41 PM, Martin Babinsky wrote:
On 04/17/2015 12:36 PM, Martin Basti wrote:
On 17/04/15 12:33, Martin Babinsky wrote:
On 04/17/2015 12:04 PM, Martin Basti wrote:
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
attempts to remove files that do not exist anymore. These errors
now logged at debug level and do not pop-up to stdout/stderr.

Hello, thank you for the patch.

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




             if file_exists(preferences_fname):
                 except OSError as e:
                     log_file_removal_error(e, preferences_fname,

In this case file not found error should never happen.

Could you remove the 'if file_exists' part and handle just

I just reverted this bit to original form in order to not fix
something that isn't broken. Is that ok?
this is inconsistent with change above, choose one style please:

             if os.path.exists(ca_file):
                 except OSError, e:
                         "Failed to remove '%s': %s", ca_file, e)

Martin Basti

Attaching updated patch.


just one nitpick, can you move the new function into installutils, it
can be used in different scripts not just in ipaclient.

I'm not sure if it is a good idea as installutils is a part for
freeipa-server package.

Placing it there would create an unnecessary dependency of
freeipa-client on freeipa-server because of a single function.

you are right, I do not why I thought that ipa-client-install uses


self-NACK, I will try to rewrite the patch in a slightly less dumb way.

Sorry for the confusion.

Attaching updated patch which does the same but using a wrapper around

Jan suggested to keep the new function in 'ipa-client-install' and
move it around when we do installer re#$%@^ing.

Is that ok?

It looks better, ACK.

Jan NACKed your ACK.

Attaching updated patch.

Sorry, NACK

************* Module ipa-client-install
ipa-client/ipa-install/ipa-client-install:791: [E1121(too-many-function-args), uninstall] Too many positional arguments for function call) ipa-client/ipa-install/ipa-client-install:797: [E1121(too-many-function-args), uninstall] Too many positional arguments for function call)

consult with Honza if option which show prompt user to delete file manually, should be there or not.

Martin Basti

Manage your subscription for the Freeipa-devel mailing list:
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to