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

[Ludwig Krispenz]

Hi,

here is the latest patch for the plugin part, trying to address all 
problems found in the review

Regards,
Ludwig
PS if you want you can get a separate diff top the last version


On 05/12/2015 08:33 AM, Ludwig Krispenz wrote:
> Hi,
>
> I did split the patches, for easier review and to share work on it.
> The attachment contains 4 patches:
> - the ds plugin part as submitted for review
> - the changes to the ds plugin part done after review  (not complete yet)
> - the ipa framework part (including Petr's improvements)
> - the install related part
>
> Regards,
> Ludwig
>
> On 04/21/2015 01:09 PM, Petr Vobornik wrote:
>> 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.
>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150519/fbc1f0c5/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-lkrispen-0003-plugin-part-manage-replication-topology-in-the-shaer.patch
Type: text/x-patch
Size: 151693 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150519/fbc1f0c5/attachment.bin>