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

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'.

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.

--
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

Reply via email to