ipalib.constants: Remove default domain, realm, basedn, xmlrpc_uri, ldap_uri #113
Conversation
…dap_uri Domain, realm, basedn, xmlrpc_uri, ldap_uri do not have any reasonable default. This patch removes hardcoded default so the so the code which depends on these values blows up early and does not do crazy stuff with default values instead of real ones. This should help to uncover issues caused by improper ipalib initialization.
There was a problem hiding this comment.
When ipa command is run on a system without ipa installed, it explodes with error that does not explain much about the situation to a normal user:
$ ipa
[2016-10-06T08:04:57Z ipa] <ERROR>: AttributeError: type object 'object' has no attribute 'find'
Traceback (most recent call last):
File "/usr/lib/python2.7/site-packages/ipalib/cli.py", line 1345, in run
(options, argv) = api.bootstrap_with_global_options(context='cli')
File "/usr/lib/python2.7/site-packages/ipalib/plugable.py", line 571, in bootstrap_with_global_options
self.bootstrap(parser, **overrides)
File "/usr/lib/python2.7/site-packages/ipalib/plugable.py", line 436, in bootstrap
self.env._finalize_core(**dict(DEFAULT_CONFIG))
File "/usr/lib/python2.7/site-packages/ipalib/config.py", line 568, in _finalize_core
parsed = urlparse(jsonrpc_uri)
File "/usr/lib64/python2.7/urlparse.py", line 143, in urlparse
tuple = urlsplit(url, scheme, allow_fragments)
File "/usr/lib64/python2.7/urlparse.py", line 182, in urlsplit
i = url.find(':')
AttributeError: type object 'object' has no attribute 'find'
While this is connected to another issue discussed in #25, I would like to see at least a temporary fix to this here.
|
NACK, please see the review comment. |
|
Please create a default basedn from realm with |
|
Actually it should be created from domain name, which is the primary identifier of an IPA domain, not from realm name. |
|
Honza will take care of this as part of ipalib cleanup for the Integration Improvements project. |
|
I would appreciate to have this fix landed in master rather sooner than later. The questionable default values have triggered hard to find bugs in one of my integration efforts. It took me a while to track them down and find the root cause. I wasted half an hour to an hour on the problem. |
|
@HonzaCholasta with @pspacek no longer caring about this PR, we should close it. But before we do it, what are your thoughts on what should be the right approach. Are you going to amend this path or replace it with something different? |
|
@pvoborni, my plan is to amend / extend this patch. |
|
Superseded by #492. |
Domain, realm, basedn, xmlrpc_uri, ldap_uri do not have any reasonable default.
This patch removes hardcoded default so the so the code which depends
on these values blows up early and does not do crazy stuff
with default values instead of real ones.
This should help to uncover issues caused by improper ipalib
initialization.
It will surely break something but right now, at the beginning of devel cycle, is IMHO the right time to do change like this and to remove some old cruft.