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

Minor install script fixes #184

Closed
wants to merge 2 commits into from
Closed

Minor install script fixes #184

wants to merge 2 commits into from

Conversation

simo5
Copy link
Contributor

@simo5 simo5 commented Oct 24, 2016

  • Use the correct unicode string for an error message, otherwise an
    exception will generate another exception about incorrect type,
    masking the original error.
  • Make sure to pass down the debug flag to ipa-client-install when
    the server install is run in debug mode

@abbra
Copy link
Contributor

abbra commented Oct 24, 2016

ACK from my side if you would split the commit into two small ones, please.

Note that CI integration is currently broken so travis says your commits failed the checks.

- Use the correct unicode string for an error message, otherwise an
exception will generate another exception about incorrect type,
masking the original error.

Signed-off-by: Simo Sorce <simo@redhat.com>
- Make sure to pass down the debug flag to ipa-client-install when
the server install is run in debug mode

Signed-off-by: Simo Sorce <simo@redhat.com>
@abbra
Copy link
Contributor

abbra commented Oct 24, 2016

ACK.

@abbra abbra added the ack Pull Request approved, can be merged label Oct 24, 2016
@MartinBasti MartinBasti added the pushed Pull Request has already been pushed label Oct 25, 2016
@pvoborni
Copy link
Member

The debug patch breaks test installation on RHEL. Following exception is printed only if install.py is adjusted to print exception on line

ipa         : ERROR    debug
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py", line 899, in install
    if options.debug:
  File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 550, in __getattr__
    raise AttributeError(name)
AttributeError: debug

I.e. there is no Knob debug nor verbose unless it is somehow copied. I don't know why it doesn't break install on Fedora but imho the patch cannot work.

This might also be the "there is no ipa-client-install.log" issue. Because it fails just before calling ipa-client-install.

@MartinBasti MartinBasti reopened this Oct 26, 2016
@martbab
Copy link
Contributor

martbab commented Oct 26, 2016

Note that the installer class has no access to debug options. These are only used in the containing ConfigureTool class to set up loggers (see https://git.fedorahosted.org/cgit/freeipa.git/tree/ipapython/install/cli.py#n267) and are not passed to the Configurable object.

We may need to revert commit #2 to unblock installation of FreeIPA server. The proper fix shall be implemented as a part of installer refactoring effort.

@abbra
Copy link
Contributor

abbra commented Oct 26, 2016

I'm fine with that (revert --debug commit). Either alternative (make Configurable be aware of the debug or do a refactoring of an installer) is roughly going into the same direction. I certainly see a reason to have Configurable gain knowledge about --debug option to allow it to configure specific services in debug mode which would go beyond just setting up loggers in the installer itself.

@martbab
Copy link
Contributor

martbab commented Oct 26, 2016

Reverted the Fix install scripts debugging commit.

master:

  • dc87300 Revert "Fix install scripts debugging"

@martbab martbab closed this Oct 26, 2016
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
5 participants