On 25.11.2015 13:49, Tomas Babej wrote:

On 11/25/2015 01:29 PM, Jan Cholasta wrote:
On 25.11.2015 13:24, Tomas Babej wrote:
On 11/10/2015 02:22 PM, Tomas Babej wrote:

This has been rarely used, and can be replaced by proper shell output


Here's an updated version of the patch that gets rid of one missed
occurrence of log_file usage.

The ticket seems unrelated to the change.

Shouldn't the option be kept in the respective commands for backward
compatibility? See how the debug option is handled in AdminTool.

Yeah, the correct ticket is: https://fedorahosted.org/freeipa/ticket/5385

I'm curious, in what manner do you envision the backward compatibility?
The debug option is being replaced with verbose, but here we're removing
existing functionality since it does not work properly and is of little
use anyway.

So there are two possiblities I see:

1.) We remove the functionality and keep the option, just to be able to
say that this option is deprecated and bail out.

2.) We keep the functionality, and keep the option, just issue a warning
when it's being used.

 From my point of view: I did not do (1), but imho we can add it, albeit
it's a marginal usability improvement. As far as (2) goes, it does not
solve the underlying problem.

Add log_file_option=False argument to add_options(), if it's True, add the option to the parser. Set it to True in commands which currently have the option.

In _setup_logging(), if the option value is not None, add a handler for the file to the log manager (untested):

    user_file_handler = dict(
ipa_log_manager.log_mgr.create_log_handlers([user_file_handler], None, None)

This way it will log into both the standard location and the user-specified file.

Jan Cholasta

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

Reply via email to