[Freeipa-devel] [freeipa PR#492][comment] [WIP] config: remove meaningless defaults
URL: https://github.com/freeipa/freeipa/pull/492 Title: #492: [WIP] config: remove meaningless defaults HonzaCholasta commented: """ I took the hard way and removed the URI argument from `ldap2.__init__()`. """ See the full comment at https://github.com/freeipa/freeipa/pull/492#issuecomment-285003106 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#492][comment] [WIP] config: remove meaningless defaults
URL: https://github.com/freeipa/freeipa/pull/492 Title: #492: [WIP] config: remove meaningless defaults tiran commented: """ It's probably easier to always define options like ```'ldap_uri``` but use ```None``` as default. ``` cd .; ./makeaci --validate ./makeaci: ipaserver/plugins/dogtag.py:244: ignoring ImportError: No module named backports_abc ./makeaci: ipaserver/plugins/dogtag.py:244: ignoring ImportError: No module named rnc2rng Traceback (most recent call last): File "./makeaci", line 134, in main(options) File "./makeaci", line 107, in main api.finalize() File "/freeipa/ipalib/plugable.py", line 747, in finalize self._get(plugin) File "/freeipa/ipalib/plugable.py", line 776, in _get instance = self.__instances[plugin] = plugin(self) File "/freeipa/ipaserver/plugins/ldap2.py", line 72, in __init__ ldap_uri = api.env.ldap_uri AttributeError: 'Env' object has no attribute 'ldap_uri' Exception AttributeError: "'ldap2' object has no attribute 'id'" in ignored make: *** [acilint] Error 1 Makefile:1108: recipe for target 'acilint' failed ``` """ See the full comment at https://github.com/freeipa/freeipa/pull/492#issuecomment-281760358 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#492][comment] [WIP] config: remove meaningless defaults
URL: https://github.com/freeipa/freeipa/pull/492 Title: #492: [WIP] config: remove meaningless defaults tiran commented: """ Can you add a comment to explain the order of checks and assignments? Without explanation, it's going to confuse the next poor developer. """ See the full comment at https://github.com/freeipa/freeipa/pull/492#issuecomment-281597346 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#492][comment] [WIP] config: remove meaningless defaults
URL: https://github.com/freeipa/freeipa/pull/492 Title: #492: [WIP] config: remove meaningless defaults HonzaCholasta commented: """ I stand corrected, but it does not make sense to reorder the code as you suggested anyway, as it would change the current default of `server` when only `xmlrpc_uri` is specified in the configuration from "use the hostname from `xmlrpc_uri`" do "do not set a default value". """ See the full comment at https://github.com/freeipa/freeipa/pull/492#issuecomment-281406314 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#492][comment] [WIP] config: remove meaningless defaults
URL: https://github.com/freeipa/freeipa/pull/492 Title: #492: [WIP] config: remove meaningless defaults tiran commented: """ It does matter. In the current version ```if 'server' not in self:``` is checked and ```self.server``` is checked a couple of lines after ```if 'ldap_uri' not in self and 'server' in self:```. """ See the full comment at https://github.com/freeipa/freeipa/pull/492#issuecomment-281379332 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#492][comment] [WIP] config: remove meaningless defaults
URL: https://github.com/freeipa/freeipa/pull/492 Title: #492: [WIP] config: remove meaningless defaults HonzaCholasta commented: """ @tiran, not really, the order does not matter here. """ See the full comment at https://github.com/freeipa/freeipa/pull/492#issuecomment-281373944 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#492][comment] [WIP] config: remove meaningless defaults
URL: https://github.com/freeipa/freeipa/pull/492 Title: #492: [WIP] config: remove meaningless defaults tiran commented: """ https://github.com/HonzaCholasta/freeipa/blob/4ebf4b907213c9951eb9cbd276e0460552563fb1/ipalib/config.py#L579 initializes server from jsonrpc_uri. Does it make sense move this block before your new code? """ See the full comment at https://github.com/freeipa/freeipa/pull/492#issuecomment-281369964 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
