Re: [Freeipa-devel] [PATCH] 0013 Fix ipa hbactest output

[Martin Basti]

On 04.08.2016 16:19, Florence Blanc-Renaud wrote:

On 08/03/2016 05:49 PM, Martin Basti wrote:



On 02.08.2016 13:22, Florence Blanc-Renaud wrote:

Hi,

please find attached a patch related to 'ipa hbactest' producing a
Traceback.

https://fedorahosted.org/freeipa/ticket/6157

Flo.



Hello Flo,


1)
can you please move that check, right bellow the for?

 for o in self.output:
+if o == 'value':
+continue

It is peformance improvements :)  We should not waste time with getting
values from dict if we will not use them

2)
 elif isinstance(result, (unicode, bool)):
 if o == 'summary':
 textui.print_summary(result)
 else:
 textui.print_indented(result)

Here you should remove the 'bool' from isinstance or update
print_indented to allow work with boolean (I prefer the first one).
Because with any other bool value it will fail again.

Thanks,
Martin^2

Hi Martin,

thanks for the review. Please find an updated patch with your comments.
Flo.

Pushed to master: cad6a551d6558441ead4b2b71d0b906ecefbdb63
ACK

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

Re: [Freeipa-devel] [PATCH] 0013 Fix ipa hbactest output

[Florence Blanc-Renaud]

On 08/03/2016 05:49 PM, Martin Basti wrote:



On 02.08.2016 13:22, Florence Blanc-Renaud wrote:

Hi,

please find attached a patch related to 'ipa hbactest' producing a
Traceback.

https://fedorahosted.org/freeipa/ticket/6157

Flo.



Hello Flo,


1)
can you please move that check, right bellow the for?

 for o in self.output:
+if o == 'value':
+continue

It is peformance improvements :)  We should not waste time with getting
values from dict if we will not use them

2)
 elif isinstance(result, (unicode, bool)):
 if o == 'summary':
 textui.print_summary(result)
 else:
 textui.print_indented(result)

Here you should remove the 'bool' from isinstance or update
print_indented to allow work with boolean (I prefer the first one).
Because with any other bool value it will fail again.

Thanks,
Martin^2

Hi Martin,

thanks for the review. Please find an updated patch with your comments.
Flo.
>From 29d13ada32b00567e2dffb632dfac827689ba475 Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud 
Date: Tue, 2 Aug 2016 10:40:54 +0200
Subject: [PATCH] Fix ipa hbactest output

ipa hbactest command produces a Traceback (TypeError: cannot concatenate
'str' and 'bool' objects)
This happens because hbactest overrides output_for_cli but does not
properly handle the output for 'value' field. 'value' contains a boolean
but it should not be displayed (refer to ipalib/frontend.py,
Command.output_for_cli()).

Note that the issue did not appear before because the 'value' field
had a flag no_display.

https://fedorahosted.org/freeipa/ticket/6157
---
 ipaclient/plugins/hbactest.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ipaclient/plugins/hbactest.py b/ipaclient/plugins/hbactest.py
index 2518719522c4eddff2e6bc341ee9a7c34b431938..1b54530b236cf654bc8ece7ab4e329850f5a6815 100644
--- a/ipaclient/plugins/hbactest.py
+++ b/ipaclient/plugins/hbactest.py
@@ -39,13 +39,15 @@ class hbactest(CommandOverride):
 # to be printed as our execute() method will return None for corresponding
 # entries and None entries will be skipped.
 for o in self.output:
+if o == 'value':
+continue
 outp = self.output[o]
 if 'no_display' in outp.flags:
 continue
 result = output[o]
 if isinstance(result, (list, tuple)):
 textui.print_attribute(unicode(outp.doc), result, '%s: %s', 1, True)
-elif isinstance(result, (unicode, bool)):
+elif isinstance(result, unicode):
 if o == 'summary':
 textui.print_summary(result)
 else:
-- 
2.7.4

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

[Freeipa-devel] [PATCH] 0013 Fix ipa hbactest output

[Florence Blanc-Renaud]

Hi,

please find attached a patch related to 'ipa hbactest' producing a 
Traceback.


https://fedorahosted.org/freeipa/ticket/6157

Flo.
>From 38a855bfc1279ae38ac3f8083903af29e9e00a8b Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud 
Date: Tue, 2 Aug 2016 10:40:54 +0200
Subject: [PATCH] Fix ipa hbactest output

ipa hbactest command produces a Traceback (TypeError: cannot concatenate
'str' and 'bool' objects)
This happens because hbactest overrides output_for_cli but does not
properly handle the output for 'value' field. 'value' contains a boolean
but it should not be displayed (refer to ipalib/frontend.py,
Command.output_for_cli()).

Note that the issue did not appear before because the 'value' field
had a flag no_display.

https://fedorahosted.org/freeipa/ticket/6157
---
 ipaclient/plugins/hbactest.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ipaclient/plugins/hbactest.py b/ipaclient/plugins/hbactest.py
index 2518719522c4eddff2e6bc341ee9a7c34b431938..36dfbdc85de9513cee12510768b5ff682c941b18 100644
--- a/ipaclient/plugins/hbactest.py
+++ b/ipaclient/plugins/hbactest.py
@@ -43,6 +43,8 @@ class hbactest(CommandOverride):
 if 'no_display' in outp.flags:
 continue
 result = output[o]
+if o == 'value':
+continue
 if isinstance(result, (list, tuple)):
 textui.print_attribute(unicode(outp.doc), result, '%s: %s', 1, True)
 elif isinstance(result, (unicode, bool)):
-- 
2.7.4

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