Is this patch to be applied on top of the vanilla upstream tree, or does
it require your previous patches applied before?

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.

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