Re: [Freeipa-devel] [PATCH] 0014 Add final debug message in installers

[Petr Viktorin]

On 04/18/2012 12:38 AM, Dmitri Pal wrote:

On 04/17/2012 01:13 PM, Petr Viktorin wrote:

On 04/17/2012 06:46 PM, John Dennis wrote:

Thank you for the explanation Petr, it's very much appreciated.

I do have a problem with this patch and I'm inclined to NAK it, but I'm
open to discussion. Here's my thoughts, if I've made mistakes in my
reasoning please point them out.

The fundamental problem is many of our command line utilities do not do
logging correctly.

Fixing the logging is not terribly difficult but it raises concerns over
invasive changes at the last minute.

To address the problem we're going to introduce what can only be called
a "hack" to compensate for the original deficiency. The hack is a bit
obscure and convoluted (and I'm not sure entirely robust). It introduces
enough complexity it's not obvious or easy to see what is going on. Code
that obscures should be treated with skepticism and be justified by
important needs. I'm also afraid what should really be a short term
work-around will get ensconced in the code and never cleaned up, it will
be duplicated, and used as an example of how things are supposed to
work.

So my question is, "Is the output of the command line utilities so
broken that it justifies introducing this?" and "Is this any less
invasive than simply fixing the messages in the utilities cleanly and
not introducing a hack?"




Yes, it's a hack, because it needs to be non-invasive. It does that by
not modifying the scripts themselves, but just wrapping them in the
context handler. So no functionality is broken, there are just
problems with extra messages printed or not printed. I think that's
the least invasive thing to do. So the answer to your second question
is yes. I'm not experienced enough to answer the first one.

I opened https://fedorahosted.org/freeipa/ticket/2652 to track the
larger issue that needs solving: removing the code duplication in
these tools. This includes a common way to configure logging and
report errors.




Let us do the hack and pick the cleanup task in 3.0 so that we have
things done correctly for future.
If this task is not enough let us open other tasks to make sure that we
track correctly the need to the right fix.
We can even revert the change in 3.0 and go a different path.




NACK

We have agreed outside this list to drop the issue for 2.2, and fix it 
properly for 3.0.


--
Petr³

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0014 Add final debug message in installers

[Dmitri Pal]

On 04/17/2012 01:13 PM, Petr Viktorin wrote:
> On 04/17/2012 06:46 PM, John Dennis wrote:
>> Thank you for the explanation Petr, it's very much appreciated.
>>
>> I do have a problem with this patch and I'm inclined to NAK it, but I'm
>> open to discussion. Here's my thoughts, if I've made mistakes in my
>> reasoning please point them out.
>>
>> The fundamental problem is many of our command line utilities do not do
>> logging correctly.
>>
>> Fixing the logging is not terribly difficult but it raises concerns over
>> invasive changes at the last minute.
>>
>> To address the problem we're going to introduce what can only be called
>> a "hack" to compensate for the original deficiency. The hack is a bit
>> obscure and convoluted (and I'm not sure entirely robust). It introduces
>> enough complexity it's not obvious or easy to see what is going on. Code
>> that obscures should be treated with skepticism and be justified by
>> important needs. I'm also afraid what should really be a short term
>> work-around will get ensconced in the code and never cleaned up, it will
>> be duplicated, and used as an example of how things are supposed to
>> work.
>>
>> So my question is, "Is the output of the command line utilities so
>> broken that it justifies introducing this?" and "Is this any less
>> invasive than simply fixing the messages in the utilities cleanly and
>> not introducing a hack?"
>>
>>
>
> Yes, it's a hack, because it needs to be non-invasive. It does that by
> not modifying the scripts themselves, but just wrapping them in the
> context handler. So no functionality is broken, there are just
> problems with extra messages printed or not printed. I think that's
> the least invasive thing to do. So the answer to your second question
> is yes. I'm not experienced enough to answer the first one.
>
> I opened https://fedorahosted.org/freeipa/ticket/2652 to track the
> larger issue that needs solving: removing the code duplication in
> these tools. This includes a common way to configure logging and
> report errors.
>
>
>
Let us do the hack and pick the cleanup task in 3.0 so that we have
things done correctly for future.
If this task is not enough let us open other tasks to make sure that we
track correctly the need to the right fix.
We can even revert the change in 3.0 and go a different path.


-- 
Thank you,
Dmitri Pal

Sr. Engineering Manager IPA project,
Red Hat Inc.


---
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0014 Add final debug message in installers

[Petr Viktorin]

On 04/17/2012 06:46 PM, John Dennis wrote:

Thank you for the explanation Petr, it's very much appreciated.

I do have a problem with this patch and I'm inclined to NAK it, but I'm
open to discussion. Here's my thoughts, if I've made mistakes in my
reasoning please point them out.

The fundamental problem is many of our command line utilities do not do
logging correctly.

Fixing the logging is not terribly difficult but it raises concerns over
invasive changes at the last minute.

To address the problem we're going to introduce what can only be called
a "hack" to compensate for the original deficiency. The hack is a bit
obscure and convoluted (and I'm not sure entirely robust). It introduces
enough complexity it's not obvious or easy to see what is going on. Code
that obscures should be treated with skepticism and be justified by
important needs. I'm also afraid what should really be a short term
work-around will get ensconced in the code and never cleaned up, it will
be duplicated, and used as an example of how things are supposed to work.

So my question is, "Is the output of the command line utilities so
broken that it justifies introducing this?" and "Is this any less
invasive than simply fixing the messages in the utilities cleanly and
not introducing a hack?"




Yes, it's a hack, because it needs to be non-invasive. It does that by 
not modifying the scripts themselves, but just wrapping them in the 
context handler. So no functionality is broken, there are just problems 
with extra messages printed or not printed. I think that's the least 
invasive thing to do. So the answer to your second question is yes. I'm 
not experienced enough to answer the first one.


I opened https://fedorahosted.org/freeipa/ticket/2652 to track the 
larger issue that needs solving: removing the code duplication in these 
tools. This includes a common way to configure logging and report errors.




--
Petr³

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0014 Add final debug message in installers

[John Dennis]

Thank you for the explanation Petr, it's very much appreciated.

I do have a problem with this patch and I'm inclined to NAK it, but I'm 
open to discussion. Here's my thoughts, if I've made mistakes in my 
reasoning please point them out.


The fundamental problem is many of our command line utilities do not do 
logging correctly.


Fixing the logging is not terribly difficult but it raises concerns over 
invasive changes at the last minute.


To address the problem we're going to introduce what can only be called 
a "hack" to compensate for the original deficiency. The hack is a bit 
obscure and convoluted (and I'm not sure entirely robust). It introduces 
enough complexity it's not obvious or easy to see what is going on. Code 
that obscures should be treated with skepticism and be justified by 
important needs. I'm also afraid what should really be a short term 
work-around will get ensconced in the code and never cleaned up, it will 
be duplicated, and used as an example of how things are supposed to work.


So my question is, "Is the output of the command line utilities so 
broken that it justifies introducing this?" and "Is this any less 
invasive than simply fixing the messages in the utilities cleanly and 
not introducing a hack?"



--
John Dennis 

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0014 Add final debug message in installers

[Petr Viktorin]

On 04/17/2012 04:12 PM, John Dennis wrote:

There have been so many versions of the patch and various comments
attached to it I'm afraid I'm still trying to wrap my head around what
the actual problem is we're trying to solve, until I have that
understanding I can't evaluate the proposed solution.

Could you please state simply what the fundamental problem is the patch
is trying to correct for and how it accomplishes it?



The fundamental problem is in 
https://fedorahosted.org/freeipa/ticket/2071: the install tools need to 
put a final message in their logs, which states whether the command 
succeeded or not, and if not, what the error was.
The patch needs to do this with minimal modifications to the scripts, so 
that it doesn't break any functionality.


It accomplishes this by wrapping the scripts' top-level code in a 
context manager. When the script is done, the context manager looks 
whether an exception was raised and logs an appropriate message.


The current problem is that some tools configure logging to the console, 
so this extra message ends up both in both the log file and on the console.
The tools themselves also write their errors to the console (via print, 
SystemExit, or traceback), so the error appears twice.


Since I do not want to add an extra message to the console, but only 
want it in a log file (if the tool uses one), I remove the console log 
handler before logging the message.



When I tried to reach this understanding myself about the only thing I
figured out was that some of our command line utilities pass a string to
the exit() command whose output is out of band with respect to logging
resulting in a duplicate message (however before application of the
patch I don't see why logging would re-emit the message).


Yes, that's exactly right. The output to console was fine, it should 
stay as it was before the patch.


--
Petr³

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0014 Add final debug message in installers

[John Dennis]

There have been so many versions of the patch and various comments 
attached to it I'm afraid I'm still trying to wrap my head around what 
the actual problem is we're trying to solve, until I have that 
understanding I can't evaluate the proposed solution.


Could you please state simply what the fundamental problem is the patch 
is trying to correct for and how it accomplishes it?


When I tried to reach this understanding myself about the only thing I 
figured out was that some of our command line utilities pass a string to 
the exit() command whose output is out of band with respect to logging 
resulting in a duplicate message (however before application of the 
patch I don't see why logging would re-emit the message).


--
John Dennis 

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0014 Add final debug message in installers

[Petr Viktorin]

On 04/17/2012 12:12 AM, Rob Crittenden wrote:

John Dennis wrote:

On 04/16/2012 04:15 PM, Rob Crittenden wrote:

John Dennis wrote:

On 04/16/2012 01:31 PM, Rob Crittenden wrote:

John Dennis wrote:

On 04/13/2012 06:25 AM, Petr Viktorin wrote:

When the utility sets logging to console, the extra log message
gets
printed out there. I agree this isn't optimal.
Attached patch removes the console log handler before logging the
result.



I read through log_manager, and found that I can do this more
cleanly
with remove_handler.

John, is this a good use of the API?


The problem you're trying to correct is that under some
circumstances a
log message is emitted twice to the console, right?

Removing the console handler fees like the wrong solution, it's a
pretty
big hammer, you have to know when to remove it and once removed any
expectation that messages will appear on the console will be untrue.

Can you give me a brief explanation as to why you're getting
duplicate
messages on the console. I wonder if there isn't a better way to
handle
the problem which isn't so invasive and potentially hidden.


The patch adds a final log message that says if the command ultimately 
succeeded or not (https://fedorahosted.org/freeipa/ticket/2071).
Some of the commands print an error message to the console 
independently. When they've configured logging to console, the added 
message appears as well.


The right thing to do is to modify the utility just log once. But this 
patch treats the utilities as black boxes, otherwise it's too invasive.



Or is the issue you don't want a console handler at all? If that's
the
case then maybe we should provide a configuration that does not
create
one, that way it will be explicit and obvious there is no console
handler.


This would mean changing how the commands set up logging. I plan to do 
that, but it's too invasive to do now.



FWIW, the current configuration of logging is historical dating
back to
the beginning of the project. When I added the log manager the intent
was to fix deficiencies in logging, not to modify the existing
behavior.
I wasn't clear to me the existing configuration was ideal. If you're
hitting problems because of the existing configuration perhaps we
should
look at the historical configuration and ask if it needs to be
modified.


This patch is standardizing logging the final disposition of a
number of
commands. Currently is must be inferred.

The problem is that some commands have had this disposition added but
open no log files so some error messages are being displayed twice and
in log format.

I think the easiest solution would simply be to scale this back to
only
those commands that open a log file at all.


You say "command" but command is a loaded word :-) I presume you mean
command line utility and not an IPA Command object. The reason I'm
drawing this distinction is because the environment Commands execute in
are not supposed to change once api.bootstrap() has completed.

Who or what is aggregating the final disposition of a number of
commands? The reason I ask is because the log_mgr object is global and
deleting a handler from a global resource to satisfy a local need may
have unintentional global side effects. How is the aggregation
accomplished?


This is a wrapper around scripts. The final log message is the last 
thing done before exiting Python.



This patch is in the context of the command-line utilities like
ipa-server-install, ipa-client-install, etc. and not the ipa tool. Some
of them initialize the api, some do not.


Also there's ipa-ldap-updater, which sometimes opens a log file and 
sometimes not.



This is exactly the type of problem I'm concerned about, a conflict over
who owns the logging configuration.

These multiple potential "owners" of a global configuration is what lead
me to introduce the "configure_state" attribute of the log manager, to
disambiguate who is in control and who initialized the configuration. In
fact bootstrap() in plugable.py explicitly examines the configure_state
attribute of the log_manager and if it's been previously configured (for
example because the api was loaded by a command line utility that has
configured it's own logging) it then defers to the logging configuration
established by the environment it's being run in (in other words if a
command line utility calls standard_logging_setup() before loading the
api it wins and the logging configuration belongs to the utility, not
the api).

All of this is to say that isolated code that reaches into a global
configuration and takes an axe to it and chops out a significant part of
the configuration that someone else may have established without them
knowing about it feels wrong.

I suspect there is some deeper problem afoot and unilaterally chopping
out the console handler is not addressing the fundamental problem and
likely introduces a new one. But I really need to look at the specific
instances of the problem, maybe if I dig through the history of this
p

Re: [Freeipa-devel] [PATCH] 0014 Add final debug message in installers

[Rob Crittenden]

John Dennis wrote:

On 04/16/2012 04:15 PM, Rob Crittenden wrote:

John Dennis wrote:

On 04/16/2012 01:31 PM, Rob Crittenden wrote:

John Dennis wrote:

On 04/13/2012 06:25 AM, Petr Viktorin wrote:

When the utility sets logging to console, the extra log message gets
printed out there. I agree this isn't optimal.
Attached patch removes the console log handler before logging the
result.



I read through log_manager, and found that I can do this more cleanly
with remove_handler.

John, is this a good use of the API?


The problem you're trying to correct is that under some
circumstances a
log message is emitted twice to the console, right?

Removing the console handler fees like the wrong solution, it's a
pretty
big hammer, you have to know when to remove it and once removed any
expectation that messages will appear on the console will be untrue.

Can you give me a brief explanation as to why you're getting duplicate
messages on the console. I wonder if there isn't a better way to
handle
the problem which isn't so invasive and potentially hidden.

Or is the issue you don't want a console handler at all? If that's the
case then maybe we should provide a configuration that does not create
one, that way it will be explicit and obvious there is no console
handler.

FWIW, the current configuration of logging is historical dating
back to
the beginning of the project. When I added the log manager the intent
was to fix deficiencies in logging, not to modify the existing
behavior.
I wasn't clear to me the existing configuration was ideal. If you're
hitting problems because of the existing configuration perhaps we
should
look at the historical configuration and ask if it needs to be
modified.


This patch is standardizing logging the final disposition of a
number of
commands. Currently is must be inferred.

The problem is that some commands have had this disposition added but
open no log files so some error messages are being displayed twice and
in log format.

I think the easiest solution would simply be to scale this back to only
those commands that open a log file at all.


You say "command" but command is a loaded word :-) I presume you mean
command line utility and not an IPA Command object. The reason I'm
drawing this distinction is because the environment Commands execute in
are not supposed to change once api.bootstrap() has completed.

Who or what is aggregating the final disposition of a number of
commands? The reason I ask is because the log_mgr object is global and
deleting a handler from a global resource to satisfy a local need may
have unintentional global side effects. How is the aggregation
accomplished?




This patch is in the context of the command-line utilities like
ipa-server-install, ipa-client-install, etc. and not the ipa tool. Some
of them initialize the api, some do not.


This is exactly the type of problem I'm concerned about, a conflict over
who owns the logging configuration.

These multiple potential "owners" of a global configuration is what lead
me to introduce the "configure_state" attribute of the log manager, to
disambiguate who is in control and who initialized the configuration. In
fact bootstrap() in plugable.py explicitly examines the configure_state
attribute of the log_manager and if it's been previously configured (for
example because the api was loaded by a command line utility that has
configured it's own logging) it then defers to the logging configuration
established by the environment it's being run in (in other words if a
command line utility calls standard_logging_setup() before loading the
api it wins and the logging configuration belongs to the utility, not
the api).

All of this is to say that isolated code that reaches into a global
configuration and takes an axe to it and chops out a significant part of
the configuration that someone else may have established without them
knowing about it feels wrong.

I suspect there is some deeper problem afoot and unilaterally chopping
out the console handler is not addressing the fundamental problem and
likely introduces a new one. But I really need to look at the specific
instances of the problem, maybe if I dig through the history of this
patch review more carefully I'll discover it, but if someone wants to
chime in an point me at the exact issue that would be great :-)


Like I've said, this is a non-issue in this case. Those utilities that 
do not open a log simply don't need to call this log_on_exit function.


Any remaining logging issues should be logged as a ticket.

I think you're overstating the logging problem in general though. 
Basically we just want a way to override the default log that the API 
chooses. So the API bootstrap() call defers to the user if logging has 
already been configured. It is the responsibility of the developer to 
know when to do what but since there is less than a half dozen 
exceptions I don't think this is worth a significant time investment.


At some point we may rewrite all of th

Re: [Freeipa-devel] [PATCH] 0014 Add final debug message in installers

[John Dennis]

On 04/16/2012 04:15 PM, Rob Crittenden wrote:

John Dennis wrote:

On 04/16/2012 01:31 PM, Rob Crittenden wrote:

John Dennis wrote:

On 04/13/2012 06:25 AM, Petr Viktorin wrote:

When the utility sets logging to console, the extra log message gets
printed out there. I agree this isn't optimal.
Attached patch removes the console log handler before logging the
result.



I read through log_manager, and found that I can do this more cleanly
with remove_handler.

John, is this a good use of the API?


The problem you're trying to correct is that under some circumstances a
log message is emitted twice to the console, right?

Removing the console handler fees like the wrong solution, it's a pretty
big hammer, you have to know when to remove it and once removed any
expectation that messages will appear on the console will be untrue.

Can you give me a brief explanation as to why you're getting duplicate
messages on the console. I wonder if there isn't a better way to handle
the problem which isn't so invasive and potentially hidden.

Or is the issue you don't want a console handler at all? If that's the
case then maybe we should provide a configuration that does not create
one, that way it will be explicit and obvious there is no console
handler.

FWIW, the current configuration of logging is historical dating back to
the beginning of the project. When I added the log manager the intent
was to fix deficiencies in logging, not to modify the existing behavior.
I wasn't clear to me the existing configuration was ideal. If you're
hitting problems because of the existing configuration perhaps we should
look at the historical configuration and ask if it needs to be modified.


This patch is standardizing logging the final disposition of a number of
commands. Currently is must be inferred.

The problem is that some commands have had this disposition added but
open no log files so some error messages are being displayed twice and
in log format.

I think the easiest solution would simply be to scale this back to only
those commands that open a log file at all.


You say "command" but command is a loaded word :-) I presume you mean
command line utility and not an IPA Command object. The reason I'm
drawing this distinction is because the environment Commands execute in
are not supposed to change once api.bootstrap() has completed.

Who or what is aggregating the final disposition of a number of
commands? The reason I ask is because the log_mgr object is global and
deleting a handler from a global resource to satisfy a local need may
have unintentional global side effects. How is the aggregation
accomplished?




This patch is in the context of the command-line utilities like
ipa-server-install, ipa-client-install, etc. and not the ipa tool. Some
of them initialize the api, some do not.


This is exactly the type of problem I'm concerned about, a conflict over 
who owns the logging configuration.


These multiple potential "owners" of a global configuration is what lead 
me to introduce the "configure_state" attribute of the log manager, to 
disambiguate who is in control and who initialized the configuration. In 
fact bootstrap() in plugable.py explicitly examines the configure_state 
attribute of the log_manager and if it's been previously configured (for 
example because the api was loaded by a command line utility that has 
configured it's own logging) it then defers to the logging configuration 
established by the environment it's being run in (in other words if a 
command line utility calls standard_logging_setup() before loading the 
api it wins and the logging configuration belongs to the utility, not 
the api).


All of this is to say that isolated code that reaches into a global 
configuration and takes an axe to it and chops out a significant part of 
the configuration that someone else may have established without them 
knowing about it feels wrong.


I suspect there is some deeper problem afoot and unilaterally chopping 
out the console handler is not addressing the fundamental problem and 
likely introduces a new one. But I really need to look at the specific 
instances of the problem, maybe if I dig through the history of this 
patch review more carefully I'll discover it, but if someone wants to 
chime in an point me at the exact issue that would be great :-)



As I said, the intention of my original request was to log the
disposition of an install command. I think this patch goes too far and
updates all admin utilities.

rob



--
John Dennis 

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0014 Add final debug message in installers

[Rob Crittenden]

John Dennis wrote:

On 04/16/2012 01:31 PM, Rob Crittenden wrote:

John Dennis wrote:

On 04/13/2012 06:25 AM, Petr Viktorin wrote:

When the utility sets logging to console, the extra log message gets
printed out there. I agree this isn't optimal.
Attached patch removes the console log handler before logging the
result.



I read through log_manager, and found that I can do this more cleanly
with remove_handler.

John, is this a good use of the API?


The problem you're trying to correct is that under some circumstances a
log message is emitted twice to the console, right?

Removing the console handler fees like the wrong solution, it's a pretty
big hammer, you have to know when to remove it and once removed any
expectation that messages will appear on the console will be untrue.

Can you give me a brief explanation as to why you're getting duplicate
messages on the console. I wonder if there isn't a better way to handle
the problem which isn't so invasive and potentially hidden.

Or is the issue you don't want a console handler at all? If that's the
case then maybe we should provide a configuration that does not create
one, that way it will be explicit and obvious there is no console
handler.

FWIW, the current configuration of logging is historical dating back to
the beginning of the project. When I added the log manager the intent
was to fix deficiencies in logging, not to modify the existing behavior.
I wasn't clear to me the existing configuration was ideal. If you're
hitting problems because of the existing configuration perhaps we should
look at the historical configuration and ask if it needs to be modified.


This patch is standardizing logging the final disposition of a number of
commands. Currently is must be inferred.

The problem is that some commands have had this disposition added but
open no log files so some error messages are being displayed twice and
in log format.

I think the easiest solution would simply be to scale this back to only
those commands that open a log file at all.


You say "command" but command is a loaded word :-) I presume you mean
command line utility and not an IPA Command object. The reason I'm
drawing this distinction is because the environment Commands execute in
are not supposed to change once api.bootstrap() has completed.

Who or what is aggregating the final disposition of a number of
commands? The reason I ask is because the log_mgr object is global and
deleting a handler from a global resource to satisfy a local need may
have unintentional global side effects. How is the aggregation
accomplished?




This patch is in the context of the command-line utilities like 
ipa-server-install, ipa-client-install, etc. and not the ipa tool. Some 
of them initialize the api, some do not.


As I said, the intention of my original request was to log the 
disposition of an install command. I think this patch goes too far and 
updates all admin utilities.


rob

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0014 Add final debug message in installers

[John Dennis]

On 04/16/2012 01:31 PM, Rob Crittenden wrote:

John Dennis wrote:

On 04/13/2012 06:25 AM, Petr Viktorin wrote:

When the utility sets logging to console, the extra log message gets
printed out there. I agree this isn't optimal.
Attached patch removes the console log handler before logging the
result.



I read through log_manager, and found that I can do this more cleanly
with remove_handler.

John, is this a good use of the API?


The problem you're trying to correct is that under some circumstances a
log message is emitted twice to the console, right?

Removing the console handler fees like the wrong solution, it's a pretty
big hammer, you have to know when to remove it and once removed any
expectation that messages will appear on the console will be untrue.

Can you give me a brief explanation as to why you're getting duplicate
messages on the console. I wonder if there isn't a better way to handle
the problem which isn't so invasive and potentially hidden.

Or is the issue you don't want a console handler at all? If that's the
case then maybe we should provide a configuration that does not create
one, that way it will be explicit and obvious there is no console handler.

FWIW, the current configuration of logging is historical dating back to
the beginning of the project. When I added the log manager the intent
was to fix deficiencies in logging, not to modify the existing behavior.
I wasn't clear to me the existing configuration was ideal. If you're
hitting problems because of the existing configuration perhaps we should
look at the historical configuration and ask if it needs to be modified.


This patch is standardizing logging the final disposition of a number of
commands. Currently is must be inferred.

The problem is that some commands have had this disposition added but
open no log files so some error messages are being displayed twice and
in log format.

I think the easiest solution would simply be to scale this back to only
those commands that open a log file at all.


You say "command" but command is a loaded word :-) I presume you mean 
command line utility and not an IPA Command object. The reason I'm 
drawing this distinction is because the environment Commands execute in 
are not supposed to change once api.bootstrap() has completed.


Who or what is aggregating the final disposition of a number of 
commands? The reason I ask is because the log_mgr object is global and 
deleting a handler from a global resource to satisfy a local need may 
have unintentional global side effects. How is the aggregation accomplished?



--
John Dennis 

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0014 Add final debug message in installers

[Rob Crittenden]

John Dennis wrote:

On 04/13/2012 06:25 AM, Petr Viktorin wrote:

When the utility sets logging to console, the extra log message gets
printed out there. I agree this isn't optimal.
Attached patch removes the console log handler before logging the
result.



I read through log_manager, and found that I can do this more cleanly
with remove_handler.

John, is this a good use of the API?


The problem you're trying to correct is that under some circumstances a
log message is emitted twice to the console, right?

Removing the console handler fees like the wrong solution, it's a pretty
big hammer, you have to know when to remove it and once removed any
expectation that messages will appear on the console will be untrue.

Can you give me a brief explanation as to why you're getting duplicate
messages on the console. I wonder if there isn't a better way to handle
the problem which isn't so invasive and potentially hidden.

Or is the issue you don't want a console handler at all? If that's the
case then maybe we should provide a configuration that does not create
one, that way it will be explicit and obvious there is no console handler.

FWIW, the current configuration of logging is historical dating back to
the beginning of the project. When I added the log manager the intent
was to fix deficiencies in logging, not to modify the existing behavior.
I wasn't clear to me the existing configuration was ideal. If you're
hitting problems because of the existing configuration perhaps we should
look at the historical configuration and ask if it needs to be modified.


This patch is standardizing logging the final disposition of a number of 
commands. Currently is must be inferred.


The problem is that some commands have had this disposition added but 
open no log files so some error messages are being displayed twice and 
in log format.


I think the easiest solution would simply be to scale this back to only 
those commands that open a log file at all.


rob

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0014 Add final debug message in installers

[John Dennis]

On 04/13/2012 06:25 AM, Petr Viktorin wrote:

When the utility sets logging to console, the extra log message gets
printed out there. I agree this isn't optimal.
Attached patch removes the console log handler before logging the result.



I read through log_manager, and found that I can do this more cleanly
with remove_handler.

John, is this a good use of the API?


The problem you're trying to correct is that under some circumstances a 
log message is emitted twice to the console, right?


Removing the console handler fees like the wrong solution, it's a pretty 
big hammer, you have to know when to remove it and once removed any 
expectation that messages will appear on the console will be untrue.


Can you give me a brief explanation as to why you're getting duplicate 
messages on the console. I wonder if there isn't a better way to handle 
the problem which isn't so invasive and potentially hidden.


Or is the issue you don't want a console handler at all? If that's the 
case then maybe we should provide a configuration that does not create 
one, that way it will be explicit and obvious there is no console handler.


FWIW, the current configuration of logging is historical dating back to 
the beginning of the project. When I added the log manager the intent 
was to fix deficiencies in logging, not to modify the existing behavior. 
I wasn't clear to me the existing configuration was ideal. If you're 
hitting problems because of the existing configuration perhaps we should 
look at the historical configuration and ask if it needs to be modified.




--
John Dennis 

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0014 Add final debug message in installers

[Petr Viktorin]

On 04/12/2012 01:30 PM, Petr Viktorin wrote:

On 04/10/2012 10:41 PM, Rob Crittenden wrote:

Petr Viktorin wrote:

On 03/30/2012 11:00 PM, Rob Crittenden wrote:

Petr Viktorin wrote:

On 03/26/2012 05:35 PM, Petr Viktorin wrote:

On 03/26/2012 04:54 PM, Rob Crittenden wrote:


Some minor compliants.



Ideally, there would be a routine that sets up the logging and
handles
command-line arguments in some uniform way (which is also needed
before
logging setup to detect ipa-server-install --uninstall).
The original patch did the common logging setup, and I hacked around
the
install/uninstall problem too.
I guess I overdid it when I simplified the patch.
I'm somewhat confused about the scope, so bear with me as I clarify
what
you mean.



If you abort the installation you get this somewhat unnerving error:

Continue to configure the system with these values? [no]:
ipa : ERROR ipa-server-install failed, SystemExit: Installation
aborted
Installation aborted

ipa-ldap-updater is the same:

# ipa-ldap-updater
[2012-03-26T14:53:41Z ipa] : ipa-ldap-updater failed,
SystemExit:
IPA is not configured on this system.
IPA is not configured on this system.

and ipa-upgradeconfig

$ ipa-upgradeconfig
[2012-03-26T14:54:05Z ipa] : ipa-upgradeconfig failed,
SystemExit:
You must be root to run this script.


You must be root to run this script.

I'm guessing that the issue is that the log file isn't opened yet.

>

It would be nice if the logging would be confined to just the log.



If I understand you correctly, the code should check if logging has
been
configured already, and if not, skip displaying the message?



When uninstalling you get the message 'ipa-server-install
successful'.
This is a little odd as well.


ipa-server-install is the name of the command. Wontfix for now,
unless
you disagree strongly.




Updated patch: only log if logging has been configured (detected by
looking at the root logger's handlers), and changed the message to
“The
ipa-server-install command has succeeded/failed”.


Works much better thanks. Just one request. When you created
final_log()
you show less information than you did in earlier patches. It is nice
seeing the SystemExit failure. Can you do something like this
(basically
cut-n-pasted from v05)?

diff --git a/ipaserver/install/installutils.py
b/ipaserver/install/installutils.
py
index 851b58d..ca82a1b 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -721,15 +721,15 @@ def script_context(operation_name):
# Only log if logging was already configured
# TODO: Do this properly (e.g. configure logging before the try/except)
if log_mgr.handlers.keys() != ['console']:
- root_logger.info(template, operation_name)
+ root_logger.info(template)
try:
yield
except BaseException, e:
if isinstance(e, SystemExit) and (e.code is None or e.code == 0):
# Not an error after all
- final_log('The %s command was successful')
+ final_log('The %s command was successful' % operation_name)
else:
- final_log('The %s command failed')
+ final_log('%s failed, %s: %s' % (operation_name, type(e).__name__,
e))
raise
else:
final_log('The %s command was successful')

This looks like:

2012-03-30T20:56:53Z INFO ipa-dns-install failed, SystemExit:
DNS is already configured in this IPA server.

rob


Fixed.



Hate to do this to you but I've found a few more issues. I basically
went down the list and ran all the commands in various conditions.

Some don't open any logs at all so the output gets written twice, like
ipa-replica-prepare and ipa-replica-manage:

# ipa-replica-manage del foo
Directory Manager password:

ipa: INFO: The ipa-replica-manage command failed, SystemExit:
'pony.greyoak.com' has no replication agreement for 'foo'
'pony.greyoak.com' has no replication agreement for 'foo'

Same with ipa-csreplica-manage.

# ipa-replica-prepare foo
Directory Manager (existing master) password:

ipa: INFO: The ipa-replica-prepare command failed, SystemExit:
The password provided is incorrect for LDAP server pony.greyoak.com

The password provided is incorrect for LDAP server pony.greyoak.com

rob


When the utility sets logging to console, the extra log message gets
printed out there. I agree this isn't optimal.
Attached patch removes the console log handler before logging the result.



I read through log_manager, and found that I can do this more cleanly 
with remove_handler.


John, is this a good use of the API?

--
Petr³
From b7fd14e300a055c59250a4c3717ff144b386d782 Mon Sep 17 00:00:00 2001
From: Petr Viktorin 
Date: Thu, 1 Mar 2012 05:29:52 -0500
Subject: [PATCH] Add final debug message in installers

Invocation of install tools is wrapped in a context manager that logs
the final success or failure.

https://fedorahosted.org/freeipa/ticket/2071
---
 install/tools/ipa-adtrust-install|   47 +++---
 install/tools/ipa-ca-install |   71 +-
 install/tools/ipa-compat-manage  |   43 ++--
 install/tools/ipa-

Re: [Freeipa-devel] [PATCH] 0014 Add final debug message in installers

[John Dennis]

On 03/30/2012 06:21 AM, Petr Viktorin wrote:

Updated patch: only log if logging has been configured (detected by
looking at the root logger's handlers), and changed the message to “The
ipa-server-install command has succeeded/failed”.


Actually the log_manager has an attribute called configure_state whose 
purpose is to tell you if the log manager has been configured (not None) 
and if so how it was configured. When it's not None it's a string whose 
value is by programmer convention (there are no restrictions). Currently 
the 3 values in use are:


'standard'
The log_mgr was initialized by standard_logging_setup()
'default'
The log_mgr was initialized by virtue of the ipa_log_manager being
loaded.
'api'
The log_mgr was initialized by the plugin bootstrap process (i.e.
the api is initialized and has the log manager bound to it).

It better to reference log_mgr.configure_state than to make assumptions 
about the internals of the class.


--
John Dennis 

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0014 Add final debug message in installers

[Petr Viktorin]

On 04/10/2012 10:41 PM, Rob Crittenden wrote:

Petr Viktorin wrote:

On 03/30/2012 11:00 PM, Rob Crittenden wrote:

Petr Viktorin wrote:

On 03/26/2012 05:35 PM, Petr Viktorin wrote:

On 03/26/2012 04:54 PM, Rob Crittenden wrote:


Some minor compliants.



Ideally, there would be a routine that sets up the logging and handles
command-line arguments in some uniform way (which is also needed
before
logging setup to detect ipa-server-install --uninstall).
The original patch did the common logging setup, and I hacked around
the
install/uninstall problem too.
I guess I overdid it when I simplified the patch.
I'm somewhat confused about the scope, so bear with me as I clarify
what
you mean.



If you abort the installation you get this somewhat unnerving error:

Continue to configure the system with these values? [no]:
ipa : ERROR ipa-server-install failed, SystemExit: Installation
aborted
Installation aborted

ipa-ldap-updater is the same:

# ipa-ldap-updater
[2012-03-26T14:53:41Z ipa] : ipa-ldap-updater failed,
SystemExit:
IPA is not configured on this system.
IPA is not configured on this system.

and ipa-upgradeconfig

$ ipa-upgradeconfig
[2012-03-26T14:54:05Z ipa] : ipa-upgradeconfig failed,
SystemExit:
You must be root to run this script.


You must be root to run this script.

I'm guessing that the issue is that the log file isn't opened yet.

>

It would be nice if the logging would be confined to just the log.



If I understand you correctly, the code should check if logging has
been
configured already, and if not, skip displaying the message?



When uninstalling you get the message 'ipa-server-install
successful'.
This is a little odd as well.


ipa-server-install is the name of the command. Wontfix for now, unless
you disagree strongly.




Updated patch: only log if logging has been configured (detected by
looking at the root logger's handlers), and changed the message to “The
ipa-server-install command has succeeded/failed”.


Works much better thanks. Just one request. When you created final_log()
you show less information than you did in earlier patches. It is nice
seeing the SystemExit failure. Can you do something like this (basically
cut-n-pasted from v05)?

diff --git a/ipaserver/install/installutils.py
b/ipaserver/install/installutils.
py
index 851b58d..ca82a1b 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -721,15 +721,15 @@ def script_context(operation_name):
# Only log if logging was already configured
# TODO: Do this properly (e.g. configure logging before the try/except)
if log_mgr.handlers.keys() != ['console']:
- root_logger.info(template, operation_name)
+ root_logger.info(template)
try:
yield
except BaseException, e:
if isinstance(e, SystemExit) and (e.code is None or e.code == 0):
# Not an error after all
- final_log('The %s command was successful')
+ final_log('The %s command was successful' % operation_name)
else:
- final_log('The %s command failed')
+ final_log('%s failed, %s: %s' % (operation_name, type(e).__name__,
e))
raise
else:
final_log('The %s command was successful')

This looks like:

2012-03-30T20:56:53Z INFO ipa-dns-install failed, SystemExit:
DNS is already configured in this IPA server.

rob


Fixed.



Hate to do this to you but I've found a few more issues. I basically
went down the list and ran all the commands in various conditions.

Some don't open any logs at all so the output gets written twice, like
ipa-replica-prepare and ipa-replica-manage:

# ipa-replica-manage del foo
Directory Manager password:

ipa: INFO: The ipa-replica-manage command failed, SystemExit:
'pony.greyoak.com' has no replication agreement for 'foo'
'pony.greyoak.com' has no replication agreement for 'foo'

Same with ipa-csreplica-manage.

# ipa-replica-prepare foo
Directory Manager (existing master) password:

ipa: INFO: The ipa-replica-prepare command failed, SystemExit:
The password provided is incorrect for LDAP server pony.greyoak.com

The password provided is incorrect for LDAP server pony.greyoak.com

rob


When the utility sets logging to console, the extra log message gets 
printed out there. I agree this isn't optimal.

Attached patch removes the console log handler before logging the result.

--
Petr³
From 92c2ec49e9be9b7f12ab0427d0bf803b582df21c Mon Sep 17 00:00:00 2001
From: Petr Viktorin 
Date: Thu, 1 Mar 2012 05:29:52 -0500
Subject: [PATCH] Add final debug message in installers

Invocation of install tools is wrapped in a context manager that logs
the final success or failure.

https://fedorahosted.org/freeipa/ticket/2071
---
 install/tools/ipa-adtrust-install|   47 +++---
 install/tools/ipa-ca-install |   71 +-
 install/tools/ipa-compat-manage  |   43 ++--
 install/tools/ipa-compliance |   17 
 install/tools/ipa-csreplica-manage   |   33 
 install/tools/ipa-dns-install|   47 +++---
 install/tool

Re: [Freeipa-devel] [PATCH] 0014 Add final debug message in installers

[Rob Crittenden]

Petr Viktorin wrote:

On 03/30/2012 11:00 PM, Rob Crittenden wrote:

Petr Viktorin wrote:

On 03/26/2012 05:35 PM, Petr Viktorin wrote:

On 03/26/2012 04:54 PM, Rob Crittenden wrote:


Some minor compliants.



Ideally, there would be a routine that sets up the logging and handles
command-line arguments in some uniform way (which is also needed before
logging setup to detect ipa-server-install --uninstall).
The original patch did the common logging setup, and I hacked around
the
install/uninstall problem too.
I guess I overdid it when I simplified the patch.
I'm somewhat confused about the scope, so bear with me as I clarify
what
you mean.



If you abort the installation you get this somewhat unnerving error:

Continue to configure the system with these values? [no]:
ipa : ERROR ipa-server-install failed, SystemExit: Installation
aborted
Installation aborted

ipa-ldap-updater is the same:

# ipa-ldap-updater
[2012-03-26T14:53:41Z ipa] : ipa-ldap-updater failed,
SystemExit:
IPA is not configured on this system.
IPA is not configured on this system.

and ipa-upgradeconfig

$ ipa-upgradeconfig
[2012-03-26T14:54:05Z ipa] : ipa-upgradeconfig failed,
SystemExit:
You must be root to run this script.


You must be root to run this script.

I'm guessing that the issue is that the log file isn't opened yet.

>

It would be nice if the logging would be confined to just the log.



If I understand you correctly, the code should check if logging has
been
configured already, and if not, skip displaying the message?



When uninstalling you get the message 'ipa-server-install successful'.
This is a little odd as well.


ipa-server-install is the name of the command. Wontfix for now, unless
you disagree strongly.




Updated patch: only log if logging has been configured (detected by
looking at the root logger's handlers), and changed the message to “The
ipa-server-install command has succeeded/failed”.


Works much better thanks. Just one request. When you created final_log()
you show less information than you did in earlier patches. It is nice
seeing the SystemExit failure. Can you do something like this (basically
cut-n-pasted from v05)?

diff --git a/ipaserver/install/installutils.py
b/ipaserver/install/installutils.
py
index 851b58d..ca82a1b 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -721,15 +721,15 @@ def script_context(operation_name):
# Only log if logging was already configured
# TODO: Do this properly (e.g. configure logging before the try/except)
if log_mgr.handlers.keys() != ['console']:
- root_logger.info(template, operation_name)
+ root_logger.info(template)
try:
yield
except BaseException, e:
if isinstance(e, SystemExit) and (e.code is None or e.code == 0):
# Not an error after all
- final_log('The %s command was successful')
+ final_log('The %s command was successful' % operation_name)
else:
- final_log('The %s command failed')
+ final_log('%s failed, %s: %s' % (operation_name, type(e).__name__,
e))
raise
else:
final_log('The %s command was successful')

This looks like:

2012-03-30T20:56:53Z INFO ipa-dns-install failed, SystemExit:
DNS is already configured in this IPA server.

rob


Fixed.



Hate to do this to you but I've found a few more issues. I basically 
went down the list and ran all the commands in various conditions.


Some don't open any logs at all so the output gets written twice, like 
ipa-replica-prepare and ipa-replica-manage:


# ipa-replica-manage del foo
Directory Manager password:

ipa: INFO: The ipa-replica-manage command failed, SystemExit: 
'pony.greyoak.com' has no replication agreement for 'foo'

'pony.greyoak.com' has no replication agreement for 'foo'

Same with ipa-csreplica-manage.

# ipa-replica-prepare foo
Directory Manager (existing master) password:

ipa: INFO: The ipa-replica-prepare command failed, SystemExit:
The password provided is incorrect for LDAP server pony.greyoak.com

The password provided is incorrect for LDAP server pony.greyoak.com

rob

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0014 Add final debug message in installers

[Petr Viktorin]

On 03/30/2012 11:00 PM, Rob Crittenden wrote:

Petr Viktorin wrote:

On 03/26/2012 05:35 PM, Petr Viktorin wrote:

On 03/26/2012 04:54 PM, Rob Crittenden wrote:


Some minor compliants.



Ideally, there would be a routine that sets up the logging and handles
command-line arguments in some uniform way (which is also needed before
logging setup to detect ipa-server-install --uninstall).
The original patch did the common logging setup, and I hacked around the
install/uninstall problem too.
I guess I overdid it when I simplified the patch.
I'm somewhat confused about the scope, so bear with me as I clarify what
you mean.



If you abort the installation you get this somewhat unnerving error:

Continue to configure the system with these values? [no]:
ipa : ERROR ipa-server-install failed, SystemExit: Installation aborted
Installation aborted

ipa-ldap-updater is the same:

# ipa-ldap-updater
[2012-03-26T14:53:41Z ipa] : ipa-ldap-updater failed,
SystemExit:
IPA is not configured on this system.
IPA is not configured on this system.

and ipa-upgradeconfig

$ ipa-upgradeconfig
[2012-03-26T14:54:05Z ipa] : ipa-upgradeconfig failed,
SystemExit:
You must be root to run this script.


You must be root to run this script.

I'm guessing that the issue is that the log file isn't opened yet.

>

It would be nice if the logging would be confined to just the log.



If I understand you correctly, the code should check if logging has been
configured already, and if not, skip displaying the message?



When uninstalling you get the message 'ipa-server-install successful'.
This is a little odd as well.


ipa-server-install is the name of the command. Wontfix for now, unless
you disagree strongly.




Updated patch: only log if logging has been configured (detected by
looking at the root logger's handlers), and changed the message to “The
ipa-server-install command has succeeded/failed”.


Works much better thanks. Just one request. When you created final_log()
you show less information than you did in earlier patches. It is nice
seeing the SystemExit failure. Can you do something like this (basically
cut-n-pasted from v05)?

diff --git a/ipaserver/install/installutils.py
b/ipaserver/install/installutils.
py
index 851b58d..ca82a1b 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -721,15 +721,15 @@ def script_context(operation_name):
# Only log if logging was already configured
# TODO: Do this properly (e.g. configure logging before the try/except)
if log_mgr.handlers.keys() != ['console']:
- root_logger.info(template, operation_name)
+ root_logger.info(template)
try:
yield
except BaseException, e:
if isinstance(e, SystemExit) and (e.code is None or e.code == 0):
# Not an error after all
- final_log('The %s command was successful')
+ final_log('The %s command was successful' % operation_name)
else:
- final_log('The %s command failed')
+ final_log('%s failed, %s: %s' % (operation_name, type(e).__name__,
e))
raise
else:
final_log('The %s command was successful')

This looks like:

2012-03-30T20:56:53Z INFO ipa-dns-install failed, SystemExit:
DNS is already configured in this IPA server.

rob


Fixed.

--
Petr³
From 7ef2bdeda3b6c9c8e9beeb56a9b4c4aae363d4dc Mon Sep 17 00:00:00 2001
From: Petr Viktorin 
Date: Thu, 1 Mar 2012 05:29:52 -0500
Subject: [PATCH] Add final debug message in installers

Invocation of install tools is wrapped in a context manager that logs
the final success or failure.

https://fedorahosted.org/freeipa/ticket/2071
---
 install/tools/ipa-adtrust-install|   47 +++---
 install/tools/ipa-ca-install |   71 +-
 install/tools/ipa-compat-manage  |   43 ++--
 install/tools/ipa-compliance |   17 
 install/tools/ipa-csreplica-manage   |   33 
 install/tools/ipa-dns-install|   47 +++---
 install/tools/ipa-ldap-updater   |   27 +++--
 install/tools/ipa-managed-entries|   35 +
 install/tools/ipa-nis-manage |   43 ++--
 install/tools/ipa-replica-conncheck  |   35 +
 install/tools/ipa-replica-install|   71 +-
 install/tools/ipa-replica-manage |   45 +++--
 install/tools/ipa-replica-prepare|   33 
 install/tools/ipa-server-certinstall |   17 
 install/tools/ipa-server-install |   66 ---
 install/tools/ipa-upgradeconfig  |   16 ---
 ipaserver/install/installutils.py|   22 ++
 17 files changed, 354 insertions(+), 314 deletions(-)

diff --git a/install/tools/ipa-adtrust-install b/install/tools/ipa-adtrust-install
index 248ea35eaa86dd59ebbc871b86df780cfd71ccf6..c3558fce0cceae879f83b61e0057d14fe42811de 100755
--- a/install/tools/ipa-adtrust-install
+++ b/install/tools/ipa-adtrust-install
@@ -227,26 +227,27 @@ def main():
 
 return 0
 
-try:
-sy

Re: [Freeipa-devel] [PATCH] 0014 Add final debug message in installers

[Rob Crittenden]

Petr Viktorin wrote:

On 03/26/2012 05:35 PM, Petr Viktorin wrote:

On 03/26/2012 04:54 PM, Rob Crittenden wrote:


Some minor compliants.



Ideally, there would be a routine that sets up the logging and handles
command-line arguments in some uniform way (which is also needed before
logging setup to detect ipa-server-install --uninstall).
The original patch did the common logging setup, and I hacked around the
install/uninstall problem too.
I guess I overdid it when I simplified the patch.
I'm somewhat confused about the scope, so bear with me as I clarify what
you mean.



If you abort the installation you get this somewhat unnerving error:

Continue to configure the system with these values? [no]:
ipa : ERROR ipa-server-install failed, SystemExit: Installation aborted
Installation aborted

ipa-ldap-updater is the same:

# ipa-ldap-updater
[2012-03-26T14:53:41Z ipa] : ipa-ldap-updater failed, SystemExit:
IPA is not configured on this system.
IPA is not configured on this system.

and ipa-upgradeconfig

$ ipa-upgradeconfig
[2012-03-26T14:54:05Z ipa] : ipa-upgradeconfig failed,
SystemExit:
You must be root to run this script.


You must be root to run this script.

I'm guessing that the issue is that the log file isn't opened yet.

>

It would be nice if the logging would be confined to just the log.



If I understand you correctly, the code should check if logging has been
configured already, and if not, skip displaying the message?



When uninstalling you get the message 'ipa-server-install successful'.
This is a little odd as well.


ipa-server-install is the name of the command. Wontfix for now, unless
you disagree strongly.




Updated patch: only log if logging has been configured (detected by
looking at the root logger's handlers), and changed the message to “The
ipa-server-install command has succeeded/failed”.


Works much better thanks. Just one request. When you created final_log() 
you show less information than you did in earlier patches. It is nice 
seeing the SystemExit failure. Can you do something like this (basically 
cut-n-pasted from v05)?


diff --git a/ipaserver/install/installutils.py 
b/ipaserver/install/installutils.

py
index 851b58d..ca82a1b 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -721,15 +721,15 @@ def script_context(operation_name):
 # Only log if logging was already configured
 # TODO: Do this properly (e.g. configure logging before the 
try/except)

 if log_mgr.handlers.keys() != ['console']:
-root_logger.info(template, operation_name)
+root_logger.info(template)
 try:
 yield
 except BaseException, e:
 if isinstance(e, SystemExit) and (e.code is None or e.code == 0):
 # Not an error after all
-final_log('The %s command was successful')
+final_log('The %s command was successful' % operation_name)
 else:
-final_log('The %s command failed')
+final_log('%s failed, %s: %s' % (operation_name, 
type(e).__name__,

e))
 raise
 else:
 final_log('The %s command was successful')

This looks like:

2012-03-30T20:56:53Z INFO ipa-dns-install failed, SystemExit:
DNS is already configured in this IPA server.

rob

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0014 Add final debug message in installers

[Petr Viktorin]

On 03/26/2012 05:35 PM, Petr Viktorin wrote:

On 03/26/2012 04:54 PM, Rob Crittenden wrote:


Some minor compliants.



Ideally, there would be a routine that sets up the logging and handles
command-line arguments in some uniform way (which is also needed before
logging setup to detect ipa-server-install --uninstall).
The original patch did the common logging setup, and I hacked around the
install/uninstall problem too.
I guess I overdid it when I simplified the patch.
I'm somewhat confused about the scope, so bear with me as I clarify what
you mean.



If you abort the installation you get this somewhat unnerving error:

Continue to configure the system with these values? [no]:
ipa : ERROR ipa-server-install failed, SystemExit: Installation aborted
Installation aborted

ipa-ldap-updater is the same:

# ipa-ldap-updater
[2012-03-26T14:53:41Z ipa] : ipa-ldap-updater failed, SystemExit:
IPA is not configured on this system.
IPA is not configured on this system.

and ipa-upgradeconfig

$ ipa-upgradeconfig
[2012-03-26T14:54:05Z ipa] : ipa-upgradeconfig failed, SystemExit:
You must be root to run this script.


You must be root to run this script.

I'm guessing that the issue is that the log file isn't opened yet.

 >

It would be nice if the logging would be confined to just the log.



If I understand you correctly, the code should check if logging has been
configured already, and if not, skip displaying the message?



When uninstalling you get the message 'ipa-server-install successful'.
This is a little odd as well.


ipa-server-install is the name of the command. Wontfix for now, unless
you disagree strongly.




Updated patch: only log if logging has been configured (detected by 
looking at the root logger's handlers), and changed the message to “The 
ipa-server-install command has succeeded/failed”.


--
Petr³
From 248744253385dab1b0747c38f37287842f00aa62 Mon Sep 17 00:00:00 2001
From: Petr Viktorin 
Date: Thu, 1 Mar 2012 05:29:52 -0500
Subject: [PATCH] Add final debug message in installers

Invocation of install tools is wrapped in a context manager that logs
the final success or failure.

https://fedorahosted.org/freeipa/ticket/2071
---
 install/tools/ipa-adtrust-install|   47 +++---
 install/tools/ipa-ca-install |   71 +-
 install/tools/ipa-compat-manage  |   43 ++--
 install/tools/ipa-compliance |   17 
 install/tools/ipa-csreplica-manage   |   33 
 install/tools/ipa-dns-install|   47 +++---
 install/tools/ipa-ldap-updater   |   27 +++--
 install/tools/ipa-managed-entries|   35 +
 install/tools/ipa-nis-manage |   43 ++--
 install/tools/ipa-replica-conncheck  |   35 +
 install/tools/ipa-replica-install|   71 +-
 install/tools/ipa-replica-manage |   45 +++--
 install/tools/ipa-replica-prepare|   33 
 install/tools/ipa-server-certinstall |   17 
 install/tools/ipa-server-install |   66 ---
 install/tools/ipa-upgradeconfig  |   16 ---
 ipaserver/install/installutils.py|   21 ++
 17 files changed, 353 insertions(+), 314 deletions(-)

diff --git a/install/tools/ipa-adtrust-install b/install/tools/ipa-adtrust-install
index 248ea35eaa86dd59ebbc871b86df780cfd71ccf6..c3558fce0cceae879f83b61e0057d14fe42811de 100755
--- a/install/tools/ipa-adtrust-install
+++ b/install/tools/ipa-adtrust-install
@@ -227,26 +227,27 @@ def main():
 
 return 0
 
-try:
-sys.exit(main())
-except SystemExit, e:
-sys.exit(e)
-except KeyboardInterrupt:
-print "Installation cancelled."
-except RuntimeError, e:
-print str(e)
-except HostnameLocalhost:
-print "The hostname resolves to the localhost address (127.0.0.1/::1)"
-print "Please change your /etc/hosts file so that the hostname"
-print "resolves to the ip address of your network interface."
-print "The KDC service does not listen on localhost"
-print ""
-print "Please fix your /etc/hosts file and restart the setup program"
-except Exception, e:
-message = "Unexpected error - see ipaserver-install.log for details:\n %s" % str(e)
-print message
-message = str(e)
-for str in traceback.format_tb(sys.exc_info()[2]):
-message = message + "\n" + str
-root_logger.debug(message)
-sys.exit(1)
+with installutils.script_context('ipa-adtrust-install'):
+try:
+sys.exit(main())
+except SystemExit, e:
+sys.exit(e)
+except KeyboardInterrupt:
+print "Installation cancelled."
+except RuntimeError, e:
+print str(e)
+except HostnameLocalhost:
+print "The hostname resolves to the localhost address (127.0.0.1/::1)"
+print "Please change your /etc/hosts file so that the hostname"
+print "resolves to the ip address of 

Re: [Freeipa-devel] [PATCH] 0014 Add final debug message in installers

[Petr Viktorin]

On 03/26/2012 04:54 PM, Rob Crittenden wrote:


Some minor compliants.



Ideally, there would be a routine that sets up the logging and handles 
command-line arguments in some uniform way (which is also needed before 
logging setup to detect ipa-server-install --uninstall).
The original patch did the common logging setup, and I hacked around the 
install/uninstall problem too.

I guess I overdid it when I simplified the patch.
I'm somewhat confused about the scope, so bear with me as I clarify what 
you mean.




If you abort the installation you get this somewhat unnerving error:

Continue to configure the system with these values? [no]:
ipa : ERROR ipa-server-install failed, SystemExit: Installation aborted
Installation aborted

ipa-ldap-updater is the same:

# ipa-ldap-updater
[2012-03-26T14:53:41Z ipa] : ipa-ldap-updater failed, SystemExit:
IPA is not configured on this system.
IPA is not configured on this system.

and ipa-upgradeconfig

$ ipa-upgradeconfig
[2012-03-26T14:54:05Z ipa] : ipa-upgradeconfig failed, SystemExit:
You must be root to run this script.


You must be root to run this script.

I'm guessing that the issue is that the log file isn't opened yet.

>

It would be nice if the logging would be confined to just the log.



If I understand you correctly, the code should check if logging has been 
configured already, and if not, skip displaying the message?




When uninstalling you get the message 'ipa-server-install successful'.
This is a little odd as well.


ipa-server-install is the name of the command. Wontfix for now, unless 
you disagree strongly.



--
Petr³

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0014 Add final debug message in installers

[Rob Crittenden]

Petr Viktorin wrote:

On 03/15/2012 11:30 AM, Petr Viktorin wrote:

On 03/01/2012 11:45 AM, Petr Viktorin wrote:

On 02/29/2012 07:46 PM, Rob Crittenden wrote:

Martin Kosek wrote:

On Mon, 2012-02-27 at 17:51 +0100, Petr Viktorin wrote:

On 02/22/2012 10:41 AM, Petr Viktorin wrote:

This fixes https://fedorahosted.org/freeipa/ticket/2071 (Add final
debug
message in installers). The try/except blocks at the end of
installers/management scripts are replaced by a call to a common
function, which includes the final message.

Obviously the installers still need some more love. This is as far
as I
got before Martin stopped me, saying I shouldn't change too much
before
a release :)


If it's still too many changes to test, I could just wrap the
blocks in
some `with add_final_message` block for now, and resubmit this patch
after the release.




Yeah, this is exactly the kind of changes that can have yet-unseen
consequences and I don't like pushing this close to the release.

The original ticket just asks for a debug message when the install
script ends. If possible, I would really prefer to have some low-risk
patch adding just those and leave install script refactoring for next
big release, like 3.x. Rob, what's your opinion on this?

Martin



Yes, I agree. Simpler is better at this point.

rob


This patch simply wraps the try blocks in a context that logs the final
result.
Most of the changes are indentation; diff with -w to see the additions.

Not sure if this would count as an update or a new patch...



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Rebased against current master.





Rebased again.


Some minor compliants.

If you abort the installation you get this somewhat unnerving error:

Continue to configure the system with these values? [no]:
ipa : ERRORipa-server-install failed, SystemExit: 
Installation aborted

Installation aborted

ipa-ldap-updater is the same:

# ipa-ldap-updater
[2012-03-26T14:53:41Z ipa] : ipa-ldap-updater failed, SystemExit: 
IPA is not configured on this system.

IPA is not configured on this system.

and ipa-upgradeconfig

$ ipa-upgradeconfig
[2012-03-26T14:54:05Z ipa] : ipa-upgradeconfig failed, SystemExit:
You must be root to run this script.


You must be root to run this script.

I'm guessing that the issue is that the log file isn't opened yet.

It would be nice if the logging would be confined to just the log.

When uninstalling you get the message 'ipa-server-install successful'. 
This is a little odd as well.


rob

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0014 Add final debug message in installers

[Petr Viktorin]

On 03/15/2012 11:30 AM, Petr Viktorin wrote:

On 03/01/2012 11:45 AM, Petr Viktorin wrote:

On 02/29/2012 07:46 PM, Rob Crittenden wrote:

Martin Kosek wrote:

On Mon, 2012-02-27 at 17:51 +0100, Petr Viktorin wrote:

On 02/22/2012 10:41 AM, Petr Viktorin wrote:

This fixes https://fedorahosted.org/freeipa/ticket/2071 (Add final
debug
message in installers). The try/except blocks at the end of
installers/management scripts are replaced by a call to a common
function, which includes the final message.

Obviously the installers still need some more love. This is as far
as I
got before Martin stopped me, saying I shouldn't change too much
before
a release :)


If it's still too many changes to test, I could just wrap the
blocks in
some `with add_final_message` block for now, and resubmit this patch
after the release.




Yeah, this is exactly the kind of changes that can have yet-unseen
consequences and I don't like pushing this close to the release.

The original ticket just asks for a debug message when the install
script ends. If possible, I would really prefer to have some low-risk
patch adding just those and leave install script refactoring for next
big release, like 3.x. Rob, what's your opinion on this?

Martin



Yes, I agree. Simpler is better at this point.

rob


This patch simply wraps the try blocks in a context that logs the final
result.
Most of the changes are indentation; diff with -w to see the additions.

Not sure if this would count as an update or a new patch...



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Rebased against current master.





Rebased again.


--
Petr³
From 444c07382eb330650106eb8e77b401e0b41aca58 Mon Sep 17 00:00:00 2001
From: Petr Viktorin 
Date: Thu, 1 Mar 2012 05:29:52 -0500
Subject: [PATCH] Add final debug message in installers

Invocation of install tools is wrapped in a context manager that logs
the final success or failure.

https://fedorahosted.org/freeipa/ticket/2071
---
 install/tools/ipa-adtrust-install|   47 +++---
 install/tools/ipa-ca-install |   71 +-
 install/tools/ipa-compat-manage  |   43 ++--
 install/tools/ipa-compliance |   17 
 install/tools/ipa-csreplica-manage   |   33 
 install/tools/ipa-dns-install|   47 +++---
 install/tools/ipa-ldap-updater   |   27 +++--
 install/tools/ipa-managed-entries|   35 +
 install/tools/ipa-nis-manage |   43 ++--
 install/tools/ipa-replica-conncheck  |   35 +
 install/tools/ipa-replica-install|   71 +-
 install/tools/ipa-replica-manage |   45 +++--
 install/tools/ipa-replica-prepare|   33 
 install/tools/ipa-server-certinstall |   17 
 install/tools/ipa-server-install |   66 ---
 install/tools/ipa-upgradeconfig  |   16 ---
 ipaserver/install/installutils.py|   16 
 17 files changed, 348 insertions(+), 314 deletions(-)

diff --git a/install/tools/ipa-adtrust-install b/install/tools/ipa-adtrust-install
index 248ea35eaa86dd59ebbc871b86df780cfd71ccf6..c3558fce0cceae879f83b61e0057d14fe42811de 100755
--- a/install/tools/ipa-adtrust-install
+++ b/install/tools/ipa-adtrust-install
@@ -227,26 +227,27 @@ def main():
 
 return 0
 
-try:
-sys.exit(main())
-except SystemExit, e:
-sys.exit(e)
-except KeyboardInterrupt:
-print "Installation cancelled."
-except RuntimeError, e:
-print str(e)
-except HostnameLocalhost:
-print "The hostname resolves to the localhost address (127.0.0.1/::1)"
-print "Please change your /etc/hosts file so that the hostname"
-print "resolves to the ip address of your network interface."
-print "The KDC service does not listen on localhost"
-print ""
-print "Please fix your /etc/hosts file and restart the setup program"
-except Exception, e:
-message = "Unexpected error - see ipaserver-install.log for details:\n %s" % str(e)
-print message
-message = str(e)
-for str in traceback.format_tb(sys.exc_info()[2]):
-message = message + "\n" + str
-root_logger.debug(message)
-sys.exit(1)
+with installutils.script_context('ipa-adtrust-install'):
+try:
+sys.exit(main())
+except SystemExit, e:
+sys.exit(e)
+except KeyboardInterrupt:
+print "Installation cancelled."
+except RuntimeError, e:
+print str(e)
+except HostnameLocalhost:
+print "The hostname resolves to the localhost address (127.0.0.1/::1)"
+print "Please change your /etc/hosts file so that the hostname"
+print "resolves to the ip address of your network interface."
+print "The KDC service does not listen on localhost"
+print ""
+print "

Re: [Freeipa-devel] [PATCH] 0014 Add final debug message in installers

[Petr Viktorin]

On 03/01/2012 11:45 AM, Petr Viktorin wrote:

On 02/29/2012 07:46 PM, Rob Crittenden wrote:

Martin Kosek wrote:

On Mon, 2012-02-27 at 17:51 +0100, Petr Viktorin wrote:

On 02/22/2012 10:41 AM, Petr Viktorin wrote:

This fixes https://fedorahosted.org/freeipa/ticket/2071 (Add final
debug
message in installers). The try/except blocks at the end of
installers/management scripts are replaced by a call to a common
function, which includes the final message.

Obviously the installers still need some more love. This is as far
as I
got before Martin stopped me, saying I shouldn't change too much
before
a release :)


If it's still too many changes to test, I could just wrap the
blocks in
some `with add_final_message` block for now, and resubmit this patch
after the release.




Yeah, this is exactly the kind of changes that can have yet-unseen
consequences and I don't like pushing this close to the release.

The original ticket just asks for a debug message when the install
script ends. If possible, I would really prefer to have some low-risk
patch adding just those and leave install script refactoring for next
big release, like 3.x. Rob, what's your opinion on this?

Martin



Yes, I agree. Simpler is better at this point.

rob


This patch simply wraps the try blocks in a context that logs the final
result.
Most of the changes are indentation; diff with -w to see the additions.

Not sure if this would count as an update or a new patch...



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Rebased against current master.

--
Petr³
From b4d10086b34b63215af1437c138d8857b2355962 Mon Sep 17 00:00:00 2001
From: Petr Viktorin 
Date: Thu, 1 Mar 2012 05:29:52 -0500
Subject: [PATCH] Add final debug message in installers

Invocation of install tools is wrapped in a context manager that logs
the final success or failure.

https://fedorahosted.org/freeipa/ticket/2071
---
 install/tools/ipa-adtrust-install|   47 +++---
 install/tools/ipa-ca-install |   71 +-
 install/tools/ipa-compat-manage  |   43 ++--
 install/tools/ipa-compliance |   17 
 install/tools/ipa-csreplica-manage   |   33 
 install/tools/ipa-dns-install|   47 +++---
 install/tools/ipa-ldap-updater   |   27 +++--
 install/tools/ipa-managed-entries|   35 +
 install/tools/ipa-nis-manage |   43 ++--
 install/tools/ipa-replica-conncheck  |   33 
 install/tools/ipa-replica-install|   71 +-
 install/tools/ipa-replica-manage |   45 +++--
 install/tools/ipa-replica-prepare|   33 
 install/tools/ipa-server-certinstall |   17 
 install/tools/ipa-server-install |   66 ---
 install/tools/ipa-upgradeconfig  |   16 ---
 ipaserver/install/installutils.py|   16 
 17 files changed, 347 insertions(+), 313 deletions(-)

diff --git a/install/tools/ipa-adtrust-install b/install/tools/ipa-adtrust-install
index 248ea35eaa86dd59ebbc871b86df780cfd71ccf6..c3558fce0cceae879f83b61e0057d14fe42811de 100755
--- a/install/tools/ipa-adtrust-install
+++ b/install/tools/ipa-adtrust-install
@@ -227,26 +227,27 @@ def main():
 
 return 0
 
-try:
-sys.exit(main())
-except SystemExit, e:
-sys.exit(e)
-except KeyboardInterrupt:
-print "Installation cancelled."
-except RuntimeError, e:
-print str(e)
-except HostnameLocalhost:
-print "The hostname resolves to the localhost address (127.0.0.1/::1)"
-print "Please change your /etc/hosts file so that the hostname"
-print "resolves to the ip address of your network interface."
-print "The KDC service does not listen on localhost"
-print ""
-print "Please fix your /etc/hosts file and restart the setup program"
-except Exception, e:
-message = "Unexpected error - see ipaserver-install.log for details:\n %s" % str(e)
-print message
-message = str(e)
-for str in traceback.format_tb(sys.exc_info()[2]):
-message = message + "\n" + str
-root_logger.debug(message)
-sys.exit(1)
+with installutils.script_context('ipa-adtrust-install'):
+try:
+sys.exit(main())
+except SystemExit, e:
+sys.exit(e)
+except KeyboardInterrupt:
+print "Installation cancelled."
+except RuntimeError, e:
+print str(e)
+except HostnameLocalhost:
+print "The hostname resolves to the localhost address (127.0.0.1/::1)"
+print "Please change your /etc/hosts file so that the hostname"
+print "resolves to the ip address of your network interface."
+print "The KDC service does not listen on localhost"
+print ""
+print "Please fix your /etc/hosts file and restart the setup program"
+

Re: [Freeipa-devel] [PATCH] 0014 Add final debug message in installers

[Petr Viktorin]

On 02/29/2012 07:46 PM, Rob Crittenden wrote:

Martin Kosek wrote:

On Mon, 2012-02-27 at 17:51 +0100, Petr Viktorin wrote:

On 02/22/2012 10:41 AM, Petr Viktorin wrote:

This fixes https://fedorahosted.org/freeipa/ticket/2071 (Add final
debug
message in installers). The try/except blocks at the end of
installers/management scripts are replaced by a call to a common
function, which includes the final message.

Obviously the installers still need some more love. This is as far as I
got before Martin stopped me, saying I shouldn't change too much before
a release :)


If it's still too many changes to test, I could just wrap the blocks in
some `with add_final_message` block for now, and resubmit this patch
after the release.




Yeah, this is exactly the kind of changes that can have yet-unseen
consequences and I don't like pushing this close to the release.

The original ticket just asks for a debug message when the install
script ends. If possible, I would really prefer to have some low-risk
patch adding just those and leave install script refactoring for next
big release, like 3.x. Rob, what's your opinion on this?

Martin



Yes, I agree. Simpler is better at this point.

rob


This patch simply wraps the try blocks in a context that logs the final 
result.

Most of the changes are indentation; diff with -w to see the additions.

Not sure if this would count as an update or a new patch...

--
Petr³
From 2c05094a2b7e272bb08dc2ab24f4b9b97b00b1b7 Mon Sep 17 00:00:00 2001
From: Petr Viktorin 
Date: Thu, 1 Mar 2012 05:29:52 -0500
Subject: [PATCH] Add final debug message in installers

Invocation of install tools is wrapped in a context manager that logs
the final success or failure.

https://fedorahosted.org/freeipa/ticket/2071
---
 install/tools/ipa-adtrust-install|   47 +++---
 install/tools/ipa-ca-install |   71 +-
 install/tools/ipa-compat-manage  |   43 ++--
 install/tools/ipa-compliance |   17 
 install/tools/ipa-csreplica-manage   |   33 
 install/tools/ipa-dns-install|   47 +++---
 install/tools/ipa-ldap-updater   |   27 +++--
 install/tools/ipa-managed-entries|   35 +
 install/tools/ipa-nis-manage |   43 ++--
 install/tools/ipa-replica-conncheck  |   33 
 install/tools/ipa-replica-install|   71 +-
 install/tools/ipa-replica-manage |   45 +++--
 install/tools/ipa-replica-prepare|   33 
 install/tools/ipa-server-certinstall |   17 
 install/tools/ipa-server-install |   66 ---
 install/tools/ipa-upgradeconfig  |   17 +---
 ipaserver/install/installutils.py|   16 
 17 files changed, 348 insertions(+), 313 deletions(-)

diff --git a/install/tools/ipa-adtrust-install b/install/tools/ipa-adtrust-install
index 248ea35eaa86dd59ebbc871b86df780cfd71ccf6..c3558fce0cceae879f83b61e0057d14fe42811de 100755
--- a/install/tools/ipa-adtrust-install
+++ b/install/tools/ipa-adtrust-install
@@ -227,26 +227,27 @@ def main():
 
 return 0
 
-try:
-sys.exit(main())
-except SystemExit, e:
-sys.exit(e)
-except KeyboardInterrupt:
-print "Installation cancelled."
-except RuntimeError, e:
-print str(e)
-except HostnameLocalhost:
-print "The hostname resolves to the localhost address (127.0.0.1/::1)"
-print "Please change your /etc/hosts file so that the hostname"
-print "resolves to the ip address of your network interface."
-print "The KDC service does not listen on localhost"
-print ""
-print "Please fix your /etc/hosts file and restart the setup program"
-except Exception, e:
-message = "Unexpected error - see ipaserver-install.log for details:\n %s" % str(e)
-print message
-message = str(e)
-for str in traceback.format_tb(sys.exc_info()[2]):
-message = message + "\n" + str
-root_logger.debug(message)
-sys.exit(1)
+with installutils.script_context('ipa-adtrust-install'):
+try:
+sys.exit(main())
+except SystemExit, e:
+sys.exit(e)
+except KeyboardInterrupt:
+print "Installation cancelled."
+except RuntimeError, e:
+print str(e)
+except HostnameLocalhost:
+print "The hostname resolves to the localhost address (127.0.0.1/::1)"
+print "Please change your /etc/hosts file so that the hostname"
+print "resolves to the ip address of your network interface."
+print "The KDC service does not listen on localhost"
+print ""
+print "Please fix your /etc/hosts file and restart the setup program"
+except Exception, e:
+message = "Unexpected error - see ipaserver-install.log for details:\n %s" % str(e)
+print message
+message = str(e)
+for str in traceback.format_tb(sys.exc_info()[2]):
+