Fix for handling CalledProcessError in authconfig #179
Conversation
| try: | ||
| ipautil.run([paths.AUTHCONFIG, "--savebackup", path]) | ||
| except ipautil.CalledProcessError: | ||
| print("Failed to execute authconfig command") |
There was a problem hiding this comment.
Can anyone suggestion, what should be more appropriate in place of print ?
There was a problem hiding this comment.
You could try to raise ScriptError(msg) and check if it is properly handled and displayed.
There was a problem hiding this comment.
I'd prefer to see the error message returned from the process instead of a generic one.
It seem that ipautil.run can return this error message when capture_errors=True. See https://github.com/freeipa/freeipa/blob/master/ipapython/ipautil.py#L322 for the relevant code.
| try: | ||
| ipautil.run([paths.AUTHCONFIG, "--savebackup", path]) | ||
| except ipautil.CalledProcessError: | ||
| print("Failed to execute authconfig command") |
There was a problem hiding this comment.
You could try to raise ScriptError(msg) and check if it is properly handled and displayed.
| try: | ||
| ipautil.run([paths.AUTHCONFIG, "--savebackup", path]) | ||
| except ipautil.CalledProcessError: | ||
| print("Failed to execute authconfig command") |
There was a problem hiding this comment.
I'd prefer to see the error message returned from the process instead of a generic one.
It seem that ipautil.run can return this error message when capture_errors=True. See https://github.com/freeipa/freeipa/blob/master/ipapython/ipautil.py#L322 for the relevant code.
|
I investigated some other options for the displayed error message, but I haven't found anything more appropriate. Comment#4 in the ticket says the message should mention an SSSD restart issue. Perhaps someone else has a suggestion for a more descriptive message then If this message is fine, the code has an ack. |
| @@ -2,7 +2,7 @@ | |||
| # Alexander Bokovoy <abokovoy@redhat.com> | |||
| # Tomas Babej <tbabej@redhat.com> | |||
| # | |||
| # Copyright (C) 2007-2014 Red Hat | |||
| # Copyright (C) 2007-2016 Red Hat, Inc. | |||
There was a problem hiding this comment.
If you're changing licence, please change it to the new format (see, e.g., ipaclient/install/client.py) + add it to a new commit.
| @@ -3,6 +3,7 @@ | |||
| # Tomas Babej <tbabej@redhat.com> | |||
| # | |||
| # Copyright (C) 2007-2014 Red Hat | |||
| # Copyright (C) 2016 FreeIPA Contributors | |||
There was a problem hiding this comment.
I don't think this is how the license should look. I'd replace the entire wall of text with just the three lines with FreeIPA Contributors, but @stlaz would probably know for sure.
Anyway, this change should not be a part of the commit that addresses 5244. Either do not touch the license information, or update to it to the current format in a separate commit.
There was a problem hiding this comment.
@tomaskrizek Ok I will remove that copyright changes.
NIS configuration error should be hidden from user while running ipa-client-install Fixes https://fedorahosted.org/freeipa/ticket/5244 Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>
|
@tomaskrizek Done. |
|
Since there's been no suggestions for a more descriptive error message -> ack. |
|
Fixed upstream |
|
@tomaskrizek @martbab Thanks for review. |
NIS configuration error should be hidden from user
while running ipa-client-install
Fixes https://fedorahosted.org/freeipa/ticket/5244
Signed-off-by: Abhijeet Kasurde akasurde@redhat.com