Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

client install: correctly report all failures #376

Closed
wants to merge 1 commit into from
Closed

client install: correctly report all failures #376

wants to merge 1 commit into from

Conversation

HonzaCholasta
Copy link
Contributor

@HonzaCholasta HonzaCholasta commented Jan 6, 2017

In commit 5249eb8, the client install code
was converted to use exception handling instead of return codes. However,
some return statements were not converted to raise statements and as a
result, ipa-client-install will report success in some error conditions.

Convert the return statements to raise statements to fix the issue.

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

In commit 5249eb8, the client install code
was converted to use exception handling instead of return codes. However,
some return statements were not converted to raise statements and as a
result, ipa-client-install will report success in some error conditions.

Convert the return statements to raise statements to fix the issue.

https://fedorahosted.org/freeipa/ticket/6392
@stlaz stlaz self-assigned this Jan 6, 2017
@stlaz
Copy link
Contributor

stlaz commented Jan 19, 2017

I suspect we are suffering the same "always return 0" error as we've already got reported in other installers, right?

@stlaz
Copy link
Contributor

stlaz commented Jan 20, 2017

Alright, that's a known issue from the refactorings. Other than that the patch is fine. ACK.

@stlaz stlaz added the ack Pull Request approved, can be merged label Jan 20, 2017
@martbab
Copy link
Contributor

martbab commented Jan 20, 2017

@martbab martbab added the pushed Pull Request has already been pushed label Jan 20, 2017
@martbab martbab closed this Jan 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged pushed Pull Request has already been pushed
Projects
None yet
3 participants