<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
<div class="moz-cite-prefix">On 05/20/2015 02:52 PM, Oleg Fayans
wrote:<br>
</div>
<blockquote cite="mid:555C8392.7010908@redhat.com" type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
Is this patch to be applied on top of the vanilla upstream tree,
or does it require your previous patches applied before? <br>
</blockquote>
it requires the install (0005) and ipa-command (0006) patch as well,
submitted on 05/12 <br>
<br>
<blockquote cite="mid:555C8392.7010908@redhat.com" type="cite"> <br>
<div class="moz-cite-prefix">On 05/19/2015 02:16 PM, Ludwig
Krispenz wrote:<br>
</div>
<blockquote cite="mid:555B2998.9020608@redhat.com" type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
Hi,<br>
<br>
here is the latest patch for the plugin part, trying to address
all problems found in the review<br>
<br>
Regards,<br>
Ludwig<br>
PS if you want you can get a separate diff top the last version<br>
<br>
<br>
<div class="moz-cite-prefix">On 05/12/2015 08:33 AM, Ludwig
Krispenz wrote:<br>
</div>
<blockquote cite="mid:55519EAC.9000204@redhat.com" type="cite">Hi,
<br>
<br>
I did split the patches, for easier review and to share work
on it. <br>
The attachment contains 4 patches: <br>
- the ds plugin part as submitted for review <br>
- the changes to the ds plugin part done after review (not
complete yet) <br>
- the ipa framework part (including Petr's improvements) <br>
- the install related part <br>
<br>
Regards, <br>
Ludwig <br>
<br>
On 04/21/2015 01:09 PM, Petr Vobornik wrote: <br>
<blockquote type="cite">On 04/21/2015 12:53 PM, Petr Vobornik
wrote: <br>
<blockquote type="cite">On 04/13/2015 10:56 AM, Ludwig
Krispenz wrote: <br>
<blockquote type="cite">Hi, <br>
<br>
in the attachment you find the latest state of the
"topology plugin", it <br>
implements what is defined in the design page: <br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://www.freeipa.org/page/V4/Manage_replication_topology">http://www.freeipa.org/page/V4/Manage_replication_topology</a>
(which is <br>
also waiting for a reviewer) <br>
<br>
It contains the plugin itself and a core of ipa
commands to manage a <br>
topology. to be really applicable, some work outside is
required, eg the <br>
management of the domain level and a decision where the
binddn group <br>
should be maintained. <br>
<br>
Thanks, <br>
Ludwig <br>
<br>
<br>
</blockquote>
<br>
I've looked at the python part, mostly because I want to
start with POC <br>
of Web UI for topology. <br>
<br>
topology.py is clearly still a work in progress. I've
reflected <br>
following comments into a patch to speed things up. <br>
<br>
What's in the patch: <br>
<br>
1. git am complains about trailing whitespaces <br>
<br>
2. pep8 check produces quite a lot of issues. New code
should be almost <br>
with any (`E501 line too long` is not a hard rule) <br>
`git diff HEAD~1 -U0 | pep8 --diff` <br>
<br>
3. some typos <br>
<br>
4. A lot of unused imports <br>
<br>
5. Option name --sname for 'Segment identifier' is not
very friendly. I <br>
don't see any examples of command options in the design
notes. <br>
<br>
6. NO_UPG_MAGIC - leftover from other plugin? <br>
<br>
7. suffix object has labels from segment <br>
<br>
8. IPA framework has a support for nested object. Key is
setting <br>
`parent_object = 'topologysuffix'` in topologysegment
object. <br>
<br>
9. repl_agmt_attrs could be in topologysegment
takes_params. <br>
<br>
10. missing various CRUD commands like topologysuffix-find
and <br>
topologysuffix-show commands. <br>
<br>
Whats missing, not fixed: <br>
1. last 2 lines of VERSION file are not updated <br>
<br>
2. Mixed terminology. Somewhere is used suffix and
somewhere replication <br>
area or just area. <br>
<br>
3. Validation <br>
- suffix should check for dn <br>
- existence of both ends of a segment <br>
<br>
4. print of segments in suffix-show needs to be improved
or removed <br>
<br>
To discuss: <br>
a) Do params in topologysegment have to have a maxlength
set? <br>
<br>
b) Terminology has to be united. Segments are nested in
suffix but <br>
sometimes are called areas and suffix is 'the suffix'.
User might be <br>
confused. E.g. shouldn't the object be named a
topologyarea instead of <br>
topologysuffix? <br>
<br>
c) I've added all missing CRUD commands. Are there any
which we don't <br>
want there, or want to restrict them. E.g. I can imagine
that deleting a <br>
suffix should be prevented if it contains any segments (or
it has to be <br>
forced (--force option)) <br>
<br>
d) Do we want to print segments in suffix-show? <br>
<br>
e) Mainly for Honza: I've added --show-segments option to
suffix-show <br>
which defaults to True. I don't like the behavior of CLI,
which asks to <br>
confirm the value all the time. My intention was to have
it there by <br>
default, but also allow to disable it by
--show-segments=False. I don't <br>
want to add it as Flag (--hide-segments) since it
restricts versatility. <br>
I would like to see an optional flag which would be filled
by default <br>
value if not explicitly defined and CLI would not ask for
the option value. <br>
<br>
<br>
</blockquote>
<br>
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. <br>
</blockquote>
<br>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
</blockquote>
<br>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
</blockquote>
<br>
<pre class="moz-signature" cols="72">--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.</pre>
</blockquote>
<br>
</body>
</html>