On 25.11.2015 14:15, Jan Cholasta wrote: > 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: >>>>> Hi, >>>>> >>>>> This has been rarely used, and can be replaced by proper shell output >>>>> redirection. >>>>> >>>>> https://fedorahosted.org/freeipa/ticket/5408 >>>>> >>>> >>>> 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.
This is such a minor thing that I would throw it away completely. It is not worth the bytes :-) User will get a message that the option does not exist which seems functionally equivalent to the 'say that this option is deprecated and bail out'. Petr^2 Spacek >> >> 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( > name='user_file', > filename=self.options.log_file, > filemode=log_file_mode, > permission=0600, > level='debug', > format=ipa_log_manager.LOGGING_FORMAT_STANDARD_FILE, > ) > 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. -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
