On 06/03/2015 01:34 PM, Petr Vobornik wrote:
On 06/03/2015 10:59 AM, Martin Babinsky wrote:
On 06/03/2015 10:52 AM, Martin Babinsky wrote:
On 05/26/2015 03:31 PM, Petr Vobornik wrote:
On 05/26/2015 12:19 PM, Petr Vobornik wrote:
this patch is based on top of my patch #856 and tbabej'
s 325-9.
Obsoletes Ludwig's 0006.
ipalib part of topology management
Design:
- http://www.freeipa.org/page/V4/Manage_replication_topology
https://fedorahosted.org/freeipa/ticket/4302
New version attached:
- domainlevel_show usage changed to domainlevel_get
- updated VERSION
- added more attrs to default_attributes
Hi Petr,
the commands themselves seem to work just fine. I had encountered some
quirks in the underlying topology plugin, but I will address them in a
different thread in order to keep the discussion relevant to the
reviewed patch.
I have some minor coomments below:
1.)
IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=121
-# Last change: pvoborni - added server-find and server-show
+IPA_API_VERSION_MINOR=122
+# Last change: pvoborni - added topology management commands
Several people were touching API in the meantime so please double-check
that you have correct VERSION and regenerate API.txt
Patch rebased.
2.)
+ Str(
+ 'nsds5replicatedattributelist?',
+ cli_name='replattrs',
+ label='Attributes to replicate',
+ doc=_('Attributes that are not replicated to a consumer
server '
+ 'during a fractional update. E.g., `(objectclass=*) '
+ '$ EXCLUDE accountlockout memberof'),
+ ),
+ Str(
+ 'nsds5replicatedattributelisttotal?',
+ cli_name='replattrstotal',
+ label=_('Attributes for total update'),
+ doc=_('Attributes that are not replicated to a consumer
server '
+ 'during a total update. E.g. (objectclass=*) $
EXCLUDE '
+ 'accountlockout'),
The descriptions of these two options confused me greatly, are these
attributes supposed to be replicated or not, or is there some more
complex logic behind them that I failed to grasp? I am cc'ing Ludwig, he
can probably explain them to us and then we can decide whether we may
alter the descriptions to be less confusing.
3.)
+ takes_params = (
+ Str(
+ 'cn',
+ cli_name='name',
+ primary_key=True,
+ label=_('Suffix name'),
+ ),
+ Str(
+ 'iparepltopoconfroot',
+ maxlength=255,
+ cli_name='suffix',
+ label=_('Suffix to be managed'),
+ normalizer=lambda value: value.lower(),
+ ),
+ )
This also confused me at first, I suggest to change the label of
'iparepltopoconfroot' to something like 'LDAP suffix to be managed' or
'LDAP subtree to be managed'.
Changed to 'LDAP suffix to be managed'
4.)
There is currently no way to rename existing topology segments/suffixes.
In the case of hosts with funky FQDN's (pointing at you, ABC lab), the
segment cn's created during replica installs are mearly impossible to
remember and it would be nice to rename them to something more
manageable. However, this is not related to core functionality and can
be a subject of a separate patch once this gets pushed.
That's all from my side.
I also forgot to ask what is the expected policy when deleting a
non-empty topology suffix. If this is not supported and you have to
first remove all segments and then the suffix itself, the
'topologysuffix-del' command should issue an error pointing the user to
correct procedure.
Do we have a use case for creation or deletion of topology suffix?
That's a good question.
Anyway, I have noticed couple more things:
1.) it seems that there some of unused imports in topology.py. Please
investigate whether all of them are really needed.
2.)
+from ipalib.plugins.baseldap import *
+from ipalib.plugins import baseldap
I do not like that starred import at all. Either import the particular
classes you use (like e.g. in basuser.py), or just leave the second
import statetement and use the appropriate namespace
(baseldap.LDAPObject etc.).
3.) there are couple of pep8 complaints, please try to fix them unless
it impairs readability:
./ipalib/constants.py:121:80: E501 line too long (81 > 79 characters)
./ipalib/plugins/topology.py:72:80: E501 line too long (88 > 79 characters)
./ipalib/plugins/topology.py:73:26: E131 continuation line unaligned for
hanging indent
./ipalib/plugins/topology.py:73:80: E501 line too long (93 > 79 characters)
./ipalib/plugins/topology.py:103:80: E501 line too long (80 > 79 characters)
./ipalib/plugins/topology.py:111:80: E501 line too long (80 > 79 characters)
./ipalib/plugins/topology.py:207:80: E501 line too long (80 > 79 characters)
./ipalib/plugins/topology.py:232:80: E501 line too long (80 > 79 characters)
./ipalib/plugins/topology.py:269:80: E501 line too long (84 > 79 characters)
./ipalib/plugins/topology.py:278:80: E501 line too long (89 > 79 characters)
./ipalib/plugins/topology.py:363:80: E501 line too long (80 > 79 characters)
./ipalib/plugins/topology.py:375:80: E501 line too long (80 > 79 characters)
--
Martin^3 Babinsky
--
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
