Re: [Freeipa-devel] [PATCH] admintool: Add error message with path to log on failure.

[Tomas Babej]

On 10/15/2015 12:33 PM, David Kupka wrote:
> Currently, when there is unhanded exception without any message,
> installer ends with error code but without any error message.
> 
> Adding line stating that some error occurred and where are logs located
> should help with debugging.
> 
> 
> 

The default value for the log_file_name is None, so we probably need to
handle this correctly in case the AdminTool class instance logs to
stderr only.

Tomas

-- 
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] admintool: Add error message with path to log on failure.

[Tomas Babej]

On 10/15/2015 01:30 PM, Tomas Babej wrote:
> 
> 
> On 10/15/2015 01:24 PM, David Kupka wrote:
>> On 15/10/15 13:02, Tomas Babej wrote:
>>>
>>>
>>> On 10/15/2015 12:33 PM, David Kupka wrote:
 Currently, when there is unhanded exception without any message,
 installer ends with error code but without any error message.

 Adding line stating that some error occurred and where are logs located
 should help with debugging.



>>>
>>> The default value for the log_file_name is None, so we probably need to
>>> handle this correctly in case the AdminTool class instance logs to
>>> stderr only.
>>>
>>> Tomas
>>>
>>
>> Thanks for good catch, fixed patch attached.
>> I guess the same is true for command_name but it is used quite
>> extensively it the code so it's probably safe to assume that it will be
>> always set.
>>
> 
> Yes, such assumption is reasonable. Unlike log_file_name, there is no
> use case for command_name being set to None intentionally.
> 
> ACK.
> 
> Tomas
> 

Pushed to master: 5aa118d14905daa8c124be1cee02cfeaf26be3e4

-- 
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] admintool: Add error message with path to log on failure.

[Tomas Babej]

On 10/15/2015 01:24 PM, David Kupka wrote:
> On 15/10/15 13:02, Tomas Babej wrote:
>>
>>
>> On 10/15/2015 12:33 PM, David Kupka wrote:
>>> Currently, when there is unhanded exception without any message,
>>> installer ends with error code but without any error message.
>>>
>>> Adding line stating that some error occurred and where are logs located
>>> should help with debugging.
>>>
>>>
>>>
>>
>> The default value for the log_file_name is None, so we probably need to
>> handle this correctly in case the AdminTool class instance logs to
>> stderr only.
>>
>> Tomas
>>
> 
> Thanks for good catch, fixed patch attached.
> I guess the same is true for command_name but it is used quite
> extensively it the code so it's probably safe to assume that it will be
> always set.
> 

Yes, such assumption is reasonable. Unlike log_file_name, there is no
use case for command_name being set to None intentionally.

ACK.

Tomas

-- 
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] admintool: Add error message with path to log on failure.

[David Kupka]

On 15/10/15 13:02, Tomas Babej wrote:



On 10/15/2015 12:33 PM, David Kupka wrote:

Currently, when there is unhanded exception without any message,
installer ends with error code but without any error message.

Adding line stating that some error occurred and where are logs located
should help with debugging.





The default value for the log_file_name is None, so we probably need to
handle this correctly in case the AdminTool class instance logs to
stderr only.

Tomas



Thanks for good catch, fixed patch attached.
I guess the same is true for command_name but it is used quite 
extensively it the code so it's probably safe to assume that it will be 
always set.


--
David Kupka
From afa43f20f8e517ce67f1c196f01c604e795e42ef Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Thu, 15 Oct 2015 12:24:34 +0200
Subject: [PATCH] admintool: Add error message with path to log on failure.

---
 ipapython/admintool.py | 4 
 1 file changed, 4 insertions(+)

diff --git a/ipapython/admintool.py b/ipapython/admintool.py
index 5aa1c19bb70f9d9049130d1e2a253abb4b86677b..e40f300b1318b7325eb2b39ec83970753d39c406 100644
--- a/ipapython/admintool.py
+++ b/ipapython/admintool.py
@@ -291,6 +291,10 @@ class AdminTool(object):
 self.command_name, type(exception).__name__, exception)
 if error_message:
 self.log.error(error_message)
+message = "The %s command failed." % self.command_name
+if self.log_file_name:
+message += " See %s for more information" % self.log_file_name
+self.log.error(message)
 
 def log_success(self):
 self.log.info('The %s command was successful', self.command_name)
-- 
2.4.3

-- 
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] admintool: Add error message with path to log on failure.

[David Kupka]

Currently, when there is unhanded exception without any message, 
installer ends with error code but without any error message.


Adding line stating that some error occurred and where are logs located 
should help with debugging.


--
David Kupka
From 15f98f44bf936434f9cbf8ab81b124cd783d3ebf Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Thu, 15 Oct 2015 12:24:34 +0200
Subject: [PATCH] admintool: Add error message with path to log on failure.

---
 ipapython/admintool.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ipapython/admintool.py b/ipapython/admintool.py
index 5aa1c19bb70f9d9049130d1e2a253abb4b86677b..a46c13e052d01caf8a53d5ac6b89dc3873085674 100644
--- a/ipapython/admintool.py
+++ b/ipapython/admintool.py
@@ -291,6 +291,8 @@ class AdminTool(object):
 self.command_name, type(exception).__name__, exception)
 if error_message:
 self.log.error(error_message)
+self.log.error("The %s command failed. See %s for more information",
+self.command_name, self.log_file_name)
 
 def log_success(self):
 self.log.info('The %s command was successful', self.command_name)
-- 
2.4.3

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