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

Wed May 20 15:40:11 UTC 2015

please find new versions of patches 0003 and 0005 for  the topology plugin.

the ds plugin patch includes
- changes to match domain level patch
- remove trailing white spaces
- use proper oids for topology schema

the install patch
- has the  70topology.ldif removed

both patches can now be applied

On 05/20/2015 03:40 PM, Ludwig Krispenz wrote:
>
> On 05/20/2015 03:07 PM, Petr Vobornik wrote:
>> On 05/20/2015 02:58 PM, Ludwig Krispenz wrote:
>>>
>>> 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
>>
>> Patch 0005 can't be applied on top of the new patch 3. Both patches 
>> contains adding of 70topology.ldif.
> ok, this was my mistake when splitting the original patch, it should 
> only be in the plugin part
> the trailing spaces in most cases are leftovers from the request to 
> make lines shorter, I will fix it for a new version
>>
>> Also please clear all trailing whitespaces from patch 3.
>>
>> $ git am 
>> freeipa-lkrispen-0003-plugin-part-manage-replication-topology-in-the-shaer.patch
>> Applying: plugin part - manage replication topology in the shaerd tree
>> /home/somebody/freeipa/.git/rebase-apply/patch:607: trailing whitespace.
>> /home/somebody/freeipa/.git/rebase-apply/patch:740: trailing whitespace.
>>      * find the attrtype and return the corresponding
>> /home/somebody/freeipa/.git/rebase-apply/patch:742: trailing whitespace.
>>      */
>> /home/somebody/freeipa/.git/rebase-apply/patch:745: trailing whitespace.
>>         /* attr is handling specific direction,
>> /home/somebody/freeipa/.git/rebase-apply/patch:772: trailing whitespace.
>> /* two static data structures to hold the
>> warning: squelched 125 whitespace errors
>> warning: 130 lines add whitespace errors.
>>
>>
>>>
>>>>
>>>> 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 --------------
A non-text attachment was scrubbed...
Name: freeipa-lkrispen-0005-install-part-manage-topology-in-shared-tree.patch
Type: text/x-patch
Size: 7678 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150520/ef5697f4/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-lkrispen-0003-ds-plugin-manage-replication-topology-in-the-shared-.patch
Type: text/x-patch
Size: 151390 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150520/ef5697f4/attachment-0001.bin>