On 03/26/2015 01:14 PM, Martin Kosek wrote:
On 03/25/2015 04:18 PM, Jan Cholasta wrote:
Hi,

Dne 25.3.2015 v 15:26 Martin Babinsky napsal(a):
The attached patch related to https://fedorahosted.org/freeipa/ticket/4931

Please make sure <https://fedorahosted.org/freeipa/ticket/3048> stays fixed.
This should be ok as we do not use ntpdate for timesync anymore (I have tested client-install with this patch in VM with quite large clock skews between client and server and it did sync just fine also with -d flag).


It is certainly not a final solution, more of an initial "hack" of sorts
just to gather some suggestions, since I am not even sure if this is the
right thing to do.

The reporter from bugzilla suggests to enable debugging of ALL commands
called through ipautil.run(), but I think that fixing all cca 157 found
usages of run() is too much work with a quite small benefit.

Anyway I would welcome some opinions about this: should the external
commands really inherit the debug settings of ipa-* utilities, and if
so, is the method showed in this patch the right way to do it?

I am not a fan of this method, ipautil.run does not know anything about the
command it runs and I think it should stay that way.

I would prefer to have an ipautil.run wrapper with debug flag using appropriate
debugging option for each command where we need to conditionally enable
debugging. Or just add the debugging option unconditionally to every command
where it could be useful.

+1, I do not like this change to ipautil.run either. It should be sole
responsibility of the caller to specify the right combinations of options,
including debug option, where applicable.


How should we attack this issue then?

If we really would do "all comands inherit the debug option", then it would be a good idea to avoid things like

    if options.debug:
        args.append('-d')

littered all over the install/upgrade/etc. code. Jan's idea of a wrapper around run() then sounds like a good move. But I suppose we would then need to store somewhere (ipaplatform?) an additional data saying which command uses which option for debugging.

(Or should we just slap on an optional '-d' passed to ntpd to quickly resolve the main issue in the ticket and worry about all of this later? ;))

--
Martin^3 Babinsky

--
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

Reply via email to