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

[Ludwig Krispenz]

On 05/20/2015 02:52 PM, Oleg Fayans wrote:
> Is this patch to be applied on top of the vanilla upstream tree, or 
> does it require your previous patches applied before?
it requires the install (0005) and ipa-command (0006) patch as well, 
submitted on 05/12

>
> On 05/19/2015 02:16 PM, Ludwig Krispenz wrote:
>> 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.
>>>
>>>
>>>
>>
>>
>>
>
> -- 
> Oleg Fayans
> Quality Engineer
> FreeIPA team
> RedHat.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150520/40364e05/attachment.htm>