[Freeipa-devel] [PATCH] manage replication topology in the shared tree

[Petr Vobornik]

On 04/21/2015 12:53 PM, Petr Vobornik wrote:
> On 04/13/2015 10:56 AM, Ludwig Krispenz wrote:
>> Hi,
>>
>> in the attachment you find the latest state of the "topology plugin", it
>> implements what is defined in the design page:
>> http://www.freeipa.org/page/V4/Manage_replication_topology (which is
>> also waiting for a reviewer)
>>
>> It contains the plugin itself and  a core of ipa commands to manage a
>> topology. to be really applicable, some work outside is required, eg the
>> management of the domain level and a decision where the binddn group
>> should be maintained.
>>
>> Thanks,
>> Ludwig
>>
>>
>
> I've looked at the python part, mostly because I want to start with POC
> of Web UI for topology.
>
> topology.py is clearly still a work in progress. I've reflected
> following comments into a patch to speed things up.
>
> What's in the patch:
>
> 1. git am complains about trailing whitespaces
>
> 2. pep8 check produces quite a lot of issues. New code should be almost
> with any (`E501 line too long` is not a hard rule)
>     `git diff HEAD~1 -U0 | pep8 --diff`
>
> 3. some typos
>
> 4. A lot of unused imports
>
> 5. Option name --sname for 'Segment identifier' is not very friendly. I
> don't see any examples of command options in the design notes.
>
> 6. NO_UPG_MAGIC - leftover from other plugin?
>
> 7. suffix object has labels from segment
>
> 8. IPA framework has a support for nested object. Key is setting
> `parent_object = 'topologysuffix'` in topologysegment object.
>
> 9. repl_agmt_attrs could be in topologysegment takes_params.
>
> 10. missing various CRUD commands like topologysuffix-find and
> topologysuffix-show commands.
>
> Whats missing, not fixed:
> 1. last 2 lines of VERSION file are not updated
>
> 2. Mixed terminology. Somewhere is used suffix and somewhere replication
> area or just area.
>
> 3. Validation
> - suffix should check for dn
> - existence of both ends of a segment
>
> 4. print of segments in suffix-show needs to be improved or removed
>
> To discuss:
> a) Do params in topologysegment have to have a maxlength set?
>
> b) Terminology has to be united. Segments are nested in suffix but
> sometimes are called areas and suffix is 'the suffix'. User might be
> confused. E.g. shouldn't the object be named a topologyarea instead of
> topologysuffix?
>
> c) I've added all missing CRUD commands. Are there any which we don't
> want there, or want to restrict them. E.g. I can imagine that deleting a
> suffix should be prevented if it contains any segments (or it has to be
> forced (--force option))
>
> d) Do we want to print segments in suffix-show?
>
> e) Mainly for Honza: I've added --show-segments option to suffix-show
> which defaults to True. I don't like the behavior of CLI, which asks to
> confirm the value all the time. My intention was to have it there by
> default, but also allow to disable it by --show-segments=False. I don't
> want to add it as Flag (--hide-segments) since it restricts versatility.
> I would like to see an optional flag which would be filled by default
> value if not explicitly defined and CLI would not ask for the option value.
>
>

Also it would be better to split the work into more patches. E.g. DS 
plugin, installation, python plugin. So ds plugin review could be 
separated from the python part.
-- 
Petr Vobornik